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

SpirePatch condition argument #113

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

GreatThane
Copy link

@GreatThane GreatThane commented Oct 28, 2018

Adds condition argument to SpirePatch, which allows a boolean value to determine whether to apply the patch. Adds the possibility to lower file sizes and load times. An example could be using Loader.DEBUG.

…o determine whether to apply the patch. Adds the possibility to lower file sizes and load times.
@kiooeht
Copy link
Owner

kiooeht commented Oct 28, 2018

What is the use of this? Your example of Loader.DEBUG won't work as it's not a compile-time constant.

@GreatThane
Copy link
Author

Oh, that's quite embarrassing. I have not worked personally with java's annotation system, I was not aware that arguments had to be compile-time constant. The thought process was to use this value to avoid a patch if it was not necessary, as a more versatile version of the "optional" argument. By using a condition at run-time, resources would not have to be used to perform a patch, which could improve loading time noticeably depending on the number of patches. For instance, if a modder had to change something for a render that could be avoided using said condition, it could preserve performance, or if multiple mods being run have similar patches, instead of having inefficient and ugly checks for other mods so errors don't occur.
For instance, if three mods were installed which all had the same fix, they would each have to check for each other. The next best option would be somehow checking if a specific method was already modified to have the same effective result as a new patch being applied, but I don't think that would be possible.

@Skrelpoid
Copy link

@GreatThane This could be possible if you use another annotation for each specific case, e.g. @DebugPatch for your Loader.Debug example. This won't be very extensible or dynamic though

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.

3 participants