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

Refactor icons as a new Resource #67

Closed
rsubtil opened this issue Jan 25, 2024 · 2 comments · Fixed by #68
Closed

Refactor icons as a new Resource #67

rsubtil opened this issue Jan 25, 2024 · 2 comments · Fixed by #68
Labels
enhancement New feature or request

Comments

@rsubtil
Copy link
Owner

rsubtil commented Jan 25, 2024

Instead of offering core nodes that automatically load a controller icon based on a path, it might be possible to, instead, offer a new Resource type, inheriting from Texture2D, that can handle all the current functionality. This is a pretty cool idea because:

  • Simplifies coding to a single implementation, instead of duplicated code that's currently on all 4 node types
  • Allows more advanced use-cases and the possibility for developers to integrate such icons on custom nodes
  • Does not require a script to be attached to the node for the icon to work
  • Does not incur on any scene changes, which can currently happen when developers switch from keyboard to controller
  • Optimization can be done more transparently to the end user (related to Optimize texture access on export #60)
  • Due to raw drawing commands, it can easily support advanced use-cases, such as multi-icon drawing (e.g. XXY - Combo Attack), and embedded in other nodes such as RichTextLabel (e.g. Press F to pay respects)

Even with this system, there are some restrictions about usability that cannot change:

  • The system must still work with plain image files like it currently does. Users need to be able to switch icon assets without additional steps.
  • The current system to accept input actions, joypad paths, and icon paths must remain unchanged in behavior to the current system.

Plus, this will result in a new breaking change, and a very substantial one, requiring either automated or manual refactoring.

@rsubtil rsubtil added the enhancement New feature or request label Jan 25, 2024
@rsubtil
Copy link
Owner Author

rsubtil commented Jan 25, 2024

Another caveat is that queuing a redraw (necessary if input method changes) is not as easy. So far the best solution is to use Resource's emit_changed, but some Godot nodes do not listen to this, notably RichTextLabel. This will be reported as a bug and hopefully be fixed.

@rsubtil
Copy link
Owner Author

rsubtil commented Jan 25, 2024

Update: The bug was reported and there's already a fix, so this won't block implementation. Will need to keep a note for any users using an older version of Godot when the fix is deployed, but other than that, this approach looks good.

@rsubtil rsubtil linked a pull request Jan 30, 2024 that will close this issue
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant