Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inspector (N changes) indicator propagates upwards #88814

Merged

Conversation

RedMser
Copy link
Contributor

@RedMser RedMser commented Feb 25, 2024

Fixes #78568

image

Sums up the number of "revertable properties" from all child sections recursively, and shows the same (N changes) marker as if it was change directly in the same section. Does not go across sub-resource boundaries for now.

@Mickeon
Copy link
Contributor

Mickeon commented Feb 25, 2024

I would hold off propagating this through subresources, at least, not at the moment.
Not sure how the PR currently handles the following scenario, but it would be a nuisance when the same Resource (with its default values changed) is reused in multiple parts.
Ideally it should only propagate when the subresource is used once, but that would feel "magical" and inconsistent on the outside.

@RedMser RedMser force-pushed the inherit-property-revert-inspector branch from 6805ab7 to 90e57aa Compare February 25, 2024 18:32
@RedMser
Copy link
Contributor Author

RedMser commented Feb 25, 2024

Added additional logic that skips propagating out of sub-inspectors.

@Mickeon
Copy link
Contributor

Mickeon commented Feb 25, 2024

Oh, so subresources being caught into the propagation were an unintentional consequence. I see.

@KoBeWi
Copy link
Member

KoBeWi commented Feb 25, 2024

The proposal was edited. Is it still resolved by this PR?
(the new content)

@RedMser
Copy link
Contributor Author

RedMser commented Feb 25, 2024

Ah, as it stands it just sums up the number of "revertable properties" from all child sections recursively, and shows the same (N changes) marker as if it was change directly in the same section. So yeah, doesn't seem like it closes the proposal.

Copy link
Contributor

@Mickeon Mickeon left a comment

Choose a reason for hiding this comment

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

Current PR's behavior is a fairly safe bet for now, should've been like this a long while ago.

editor/editor_inspector.cpp Outdated Show resolved Hide resolved
@RedMser RedMser force-pushed the inherit-property-revert-inspector branch from 90e57aa to 7a08b1f Compare February 25, 2024 20:12
@akien-mga akien-mga modified the milestones: 4.x, 4.3 Feb 26, 2024
@akien-mga akien-mga merged commit a3b44bd into godotengine:master Feb 26, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parent categories doesn't show changes
5 participants