Skip to content

Editor: ActorEditLogic: support for dynamic generation of items in dropdown#21274

Merged
PunkPun merged 1 commit intoOpenRA:bleedfrom
michaeldgg2:ActorEditLogic-DropdownDynamicItems
Sep 20, 2024
Merged

Editor: ActorEditLogic: support for dynamic generation of items in dropdown#21274
PunkPun merged 1 commit intoOpenRA:bleedfrom
michaeldgg2:ActorEditLogic-DropdownDynamicItems

Conversation

@michaeldgg2
Copy link
Copy Markdown
Contributor

This PR makes it possible for EditorActorDropdown to dynamically generate items for particular actor.

This requires second constructor for EditorActorDropdown to keep backward-compatibility with existing traits (which will still use old callbacks GetLabels and GetValue).

Comment thread OpenRA.Mods.Common/Widgets/Logic/Editor/ActorEditLogic.cs Outdated
@pchote
Copy link
Copy Markdown
Member

pchote commented Dec 27, 2023

IMO adding a new constructor just for downstream use isn't a great idea - it is likely to break or be removed in the future. If there's a good motivation for per-actor items then we should make the default logic use the same code paths.

@michaeldgg2
Copy link
Copy Markdown
Contributor Author

Yes, it might seem it's somewhat specific to our mod, but this can be used any other mod. But I agree that there might be a better solution for our specific use case.

I'm reposting two more solutions from Discord here:

Another solution that I can think of is to create a completely separate IEditorActorOptions object and then widget for it - just for actor references. The trait creating IEditorActorOptions could then customize, which actors to show, or other stuff. But I believe this would be even more specific to our situation.

Yet another solution: make it possible to create entirely custom IEditorActorOptions in mod code, i.e. insert extensibility point in ActorEditLogic, which could add widgets into initContainer. This would make editor more extensible and customizable by a mod, but it would require some more work to hook it up into the infrastructure of ActorEditLogic.

Copy link
Copy Markdown
Member

@RoosterDragon RoosterDragon left a comment

Choose a reason for hiding this comment

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

This changes seems reasonable to me. But agree with pchote's comment. Remove the old constructor and have all existing callsites call your new, more general constructor.

@michaeldgg2 michaeldgg2 force-pushed the ActorEditLogic-DropdownDynamicItems branch from bf11378 to 5fc5699 Compare September 14, 2024 15:24
@PunkPun PunkPun merged commit 073ce4a into OpenRA:bleed Sep 20, 2024
@PunkPun
Copy link
Copy Markdown
Member

PunkPun commented Sep 20, 2024

changelog

@michaeldgg2 michaeldgg2 deleted the ActorEditLogic-DropdownDynamicItems branch September 20, 2024 20:50
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.

5 participants