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

Simplify and clean up resource loading #132

Merged
merged 9 commits into from
Jan 22, 2025
Merged

Conversation

StenAL
Copy link

@StenAL StenAL commented Jan 21, 2025

This PR contains a series of fix-ups and simplifications for the resource loader classes TextManager, ImageManager, and SoundManager

  • Handling of shared and game resources is combined -- there is no need to distinguish shared resources since this project is focused only on the Minigolf game.
  • Resources are loaded in the main thread. Since all resources are present in the client jar and not fetched remotely, it is okay to load them synchronously in the main thread. I tried measuring if this had any effect on start-up performance and did not see any differences
  • XML is parsed using Java standard library APIs instead of having a custom parser
    • The LocalizationNode class representing XML elements is removed. The codebase only tried to read the singular subelement in it and never accessed zero or plural so those translations have been removed along with the class. Now the XML translation just contain strings inside str elements.
  • Methods taking an optional number of parameters (getText in TextManager) now use varargs instead of defining 6 different methods for each possible number of parameters

There is also a small bugfix included to remove debug colors I added to RoundedUpperCornersButton when deobfuscating it.

These were accidentally left over when deobfuscating the class.
This made me realize that this class just does standard XML parsing and
can be removed in favor of Java's built-in XML-manipulation APIs.
Instead of using a custom parser, the built-in XML parser is more mature
and tested. This enables removing ~500 lines of code that is no longer
needed.
Since this repository only contains the golf game, there is no purpose
to handle image loading in two separate ways. Instead all image loading
now takes the same code path.
There was a lot duplicated and/or unused code in TextManager previously.
Now the API surface is smaller and easier to reason about.
Since the assets are not fetched remotely, loading them from RAM is
near-instant and does not need threading. This allows to simplify the
TextManager code.
The code never tries to read any texts with quantities other than 1 so
there is no need for the zero or plural translations. Without these
cases, each LocalizationNode is essentially just a string and so the
whole class can be removed and TextManager can directly work with
strings instead.
@PhilippvK PhilippvK merged commit a2f486b into PhilippvK:master Jan 22, 2025
2 checks passed
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