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

Add PropertyListHelper in all simple cases #88306

Merged

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Feb 14, 2024

Follow-up to #84635

This adds PropertyListHelper in simple cases, i.e. I just tried to add it as is, with minimal modifications to the helper class. Unfortunately I still had to do some minor changes due to property shenanigans in OptionButton and MenuButton.

This adds PropertyListHelper to TileMap, FileDialog, MenuButton, OptionButton, TabBar and AudioStreamRandomizer.

The remaining classes that could use it are Curve/2D/3D, TileSet and some tile editors. Curves have conditional properties and TileSet has multiple property patterns; handling them might require some more changes to the helper class. Tile editors are probably not worth changing.

EDIT:
I had to move initialization of TileMap's helper, because TileMapLayer inside _bind_methods() causes problems: https://github.com/godotengine/godot/actions/runs/7894492122/job/21545261074?pr=88306

@KoBeWi KoBeWi added this to the 4.3 milestone Feb 14, 2024
@KoBeWi KoBeWi requested review from a team as code owners February 14, 2024 00:12
@KoBeWi KoBeWi force-pushed the PropertyListHelper_will_come_for_you branch 3 times, most recently from 8ef6cbb to 78e9c52 Compare February 14, 2024 17:42
@KoBeWi KoBeWi force-pushed the PropertyListHelper_will_come_for_you branch 2 times, most recently from 17acdee to 3247e69 Compare February 29, 2024 21:06
@KoBeWi KoBeWi force-pushed the PropertyListHelper_will_come_for_you branch from 3247e69 to 7b7c102 Compare March 31, 2024 22:55
@akien-mga akien-mga merged commit 043ca7c into godotengine:master Apr 18, 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.

2 participants