Skip to content

Allow to override behavior properties on object instances#8171

Merged
4ian merged 33 commits intomasterfrom
instance-behavior
Feb 9, 2026
Merged

Allow to override behavior properties on object instances#8171
4ian merged 33 commits intomasterfrom
instance-behavior

Conversation

@D8H
Copy link
Copy Markdown
Collaborator

@D8H D8H commented Jan 26, 2026

No description provided.

@D8H D8H changed the title Instance behavior Allow to override behavior properties on object instances Jan 26, 2026
@D8H D8H force-pushed the instance-behavior branch 6 times, most recently from 72dc876 to a992e53 Compare February 2, 2026 10:47
@D8H D8H marked this pull request as ready for review February 2, 2026 13:37
@D8H D8H requested a review from 4ian as a code owner February 2, 2026 13:37
@4ian
Copy link
Copy Markdown
Owner

4ian commented Feb 2, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines 45 to +49
): CustomObjectConfiguration_EdgeAnchor =>
properties.has(name)
overridingProperties && overridingProperties.has(name)
? gd.CustomObjectConfiguration.getEdgeAnchorFromString(
overridingProperties.get(name).getValue()
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@D8H D8H force-pushed the instance-behavior branch from 49d2933 to 041ef94 Compare February 3, 2026 16:21
Comment thread Core/GDCore/Project/InitialInstance.cpp Outdated

GetVariables().SerializeTo(element.AddChild("initialVariables"));

// TODO Don't serialize empty overridings
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this TODO still relevant?

double height; ///< Instance custom height
double depth; ///< Instance custom depth
gd::String objectName; ///< Object name
double x = 0; ///< Instance X position
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice :)

/>
}
labelColor={
isFieldShowcased({ instances, field }) ? 'primary' : 'secondary'
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Highlighted?

tileMapTileSelection={null}
onSelectTileMapTile={() => {}}
isVariableListLocked={false}
canOverrideBehaviors={true}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why we should do this?

@D8H D8H force-pushed the instance-behavior branch from 32eaa17 to 6ee543e Compare February 5, 2026 19:18
behaviorType, oldPropertyName, newPropertyName);
projectBrowser.ExposeObjects(project, behaviorPropertyRenamer);

// TODO Rename the property in behavior overridings of InitialInstances.
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@D8H should this be done or not?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@4ian
Copy link
Copy Markdown
Owner

4ian commented Feb 9, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines 690 to 694
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.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@4ian 4ian merged commit deb802c into master Feb 9, 2026
6 checks passed
@4ian 4ian deleted the instance-behavior branch February 9, 2026 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants