Allow to override behavior properties on object instances#8171
Conversation
72dc876 to
a992e53
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a992e53502
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ): CustomObjectConfiguration_EdgeAnchor => | ||
| properties.has(name) | ||
| overridingProperties && overridingProperties.has(name) | ||
| ? gd.CustomObjectConfiguration.getEdgeAnchorFromString( | ||
| overridingProperties.get(name).getValue() | ||
| ) |
There was a problem hiding this comment.
Fall back to object values when no override exists
When an instance has an Anchor behavior overriding, getPropertyValue always prefers overridingProperties as long as the map has the key. But getProperties() returns entries for all properties even when none were overridden (defaults are derived from empty content). That means any instance that overrides only one Anchor property will silently reset the other Anchor properties to behavior defaults, ignoring the object-level values. This changes layouting for custom objects whenever any Anchor override exists. Consider checking behaviorOverriding.hasPropertyValue(name) (as done in the properties editor) before using the override, or pass the overriding behavior and query per-property so non-overridden values fall back to properties.
Useful? React with 👍 / 👎.
49d2933 to
041ef94
Compare
|
|
||
| GetVariables().SerializeTo(element.AddChild("initialVariables")); | ||
|
|
||
| // TODO Don't serialize empty overridings |
| double height; ///< Instance custom height | ||
| double depth; ///< Instance custom depth | ||
| gd::String objectName; ///< Object name | ||
| double x = 0; ///< Instance X position |
| /> | ||
| } | ||
| labelColor={ | ||
| isFieldShowcased({ instances, field }) ? 'primary' : 'secondary' |
| tileMapTileSelection={null} | ||
| onSelectTileMapTile={() => {}} | ||
| isVariableListLocked={false} | ||
| canOverrideBehaviors={true} |
There was a problem hiding this comment.
Nitpicking but I would prefer we call this canOverrideBehaviorProperties everywhere. It's more explicit and correct.
| const getDescription = () => propertyDescription; | ||
|
|
||
| const measurementUnit = property.getMeasurementUnit(); | ||
| // TODO Pass this object in the schema instead of building an UI element here. |
There was a problem hiding this comment.
Can you explain why we should do this?
32eaa17 to
6ee543e
Compare
| behaviorType, oldPropertyName, newPropertyName); | ||
| projectBrowser.ExposeObjects(project, behaviorPropertyRenamer); | ||
|
|
||
| // TODO Rename the property in behavior overridings of InitialInstances. |
There was a problem hiding this comment.
I think it can be done later. The refactoring operation will be scoped to object variant (to avoid going though all instances of all scenes) and I'm not sure we already handle this.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8bd4a24d87
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!cache || cache.length === 0) { | ||
| obj = new ctor(this, objectData); | ||
| obj = new ctor(this, objectData, instanceData); | ||
| } else { | ||
| // Reuse an objet destroyed before. If there is an object in the cache, | ||
| // then it means it does support reinitialization. |
There was a problem hiding this comment.
Apply instance behavior overrides when reusing cached objects
When createObject reuses a cached object, it only calls reinitialize(objectData) and never reapplies the instanceData overrides that are now passed for initial instances. This means any instance-specific behavior overrides (from instanceData.behaviorOverridings) will be silently dropped if an object is created while a cached instance exists (e.g., after objects have been destroyed/recycled in the same scene, or when spawning external layouts later). Consider extending reinitialize to accept instanceData or reapply overrides after reinitialization so cached objects behave like freshly constructed ones.
Useful? React with 👍 / 👎.
No description provided.