[GameStudio] FIX for #1913 - Make "Add Component" button functional with multiple entities selected#3089
[GameStudio] FIX for #1913 - Make "Add Component" button functional with multiple entities selected#3089luca-domenichini wants to merge 3 commits intostride3d:masterfrom
Conversation
…ultiple entities selected
|
@dotnet-policy-service agree |
|
question for maintainers (tagging a random one @VaclavElias) I changed a public facing method I am open to do the following, as maintainers prefer:
|
|
I'm not a mantainer, but imho (talking from my understanding, I have not checked if this makes sense):
Does this make sense? |
Actually, it already was
Yes, done 👍. In the meanwhile, let me refactor with your suggestions.. |
|
From a very quick search across all the repo, that method is only used in |
…ed old CombineProperty non generic method marked as Obsolete
There was a problem hiding this comment.
Pull request overview
Fixes GameStudio multi-selection behavior so the “Add Component” popup works when multiple entities are selected by ensuring combined attached-property values keep the correct generic type for WPF bindings.
Changes:
- Introduces a generic
CombineProperties<TAbstractNodeEntry>combiner to preserve the concrete entry type when combining attached properties. - Switches entity component “available types”/“type groups” attached-property keys to use the generic combiner.
- Keeps the previous non-generic combiner as
[Obsolete]for backward compatibility.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| sources/editor/Stride.Core.Assets.Editor/Quantum/NodePresenters/Keys/AbstractNodeEntryData.cs | Adds a generic property combiner to avoid WPF binding type-coercion issues when combining attached properties. |
| sources/editor/Stride.Assets.Presentation/NodePresenters/Keys/EntityHierarchyData.cs | Uses the generic combiner for component type/type-group attached properties so multi-selection keeps the expected types. |
Comments suppressed due to low confidence (1)
sources/editor/Stride.Core.Assets.Editor/Quantum/NodePresenters/Keys/AbstractNodeEntryData.cs:21
resultis initialized to a newHashSet<AbstractNodeEntry>()and then immediately overwritten withhashSets[0]. Removing the unused allocation will make the code clearer and avoid an extra allocation.
var result = new HashSet<AbstractNodeEntry>();
var hashSets = new List<HashSet<AbstractNodeEntry>>();
hashSets.AddRange(properties.Cast<IEnumerable<AbstractNodeEntry>>().Select(x => new HashSet<AbstractNodeEntry>(x)));
result = hashSets[0];
You can also share your feedback on Copilot code review. Take the survey.
| /// <param name="properties"></param> | ||
| /// <typeparam name="TAbstractNodeEntry"></typeparam> | ||
| /// <returns></returns> |
sources/editor/Stride.Core.Assets.Editor/Quantum/NodePresenters/Keys/AbstractNodeEntryData.cs
Outdated
Show resolved
Hide resolved
| [Obsolete("Use the generic version of CombineProperties instead, which allows to specify the type of the properties to combine. This method is kept for backward compatibility, but it is recommended to use the generic version instead.")] | ||
| public static object CombineProperty(IEnumerable<object> properties) | ||
| { |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
PR Details
This PR fixes a bug not allowing users to add components to multple entities at once. The popup for component addition was not showing.
Related Issue
#1913
Types of changes
Checklist