-
Notifications
You must be signed in to change notification settings - Fork 8
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
Data Driving #25
Data Driving #25
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a nitpick, question, and a major change suggestion, the actual deferred registration code looks great though
src/main/java/io/github/debuggyteam/architecture_extensions/api/BlockType.java
Show resolved
Hide resolved
//ArchitectureExtensions.LOGGER.info("Deferred generation: "+deferral.modId()+" requested "+deferral.getIds()+" and registration was deferred."); | ||
deferrals.put(groupedBlock.id(), deferral); | ||
} else { | ||
//ArchitectureExtensions.LOGGER.info("Deferred generation: "+deferral.modId()+" requested "+deferral.getIds()+" and registration was completed immediately."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are these commented out code lines still here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to help future debuggers
src/main/java/io/github/debuggyteam/architecture_extensions/DeferredRegistration.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me aside from a few things I spotted.
An aside: If you'd like, you could throw in some more documentation so whoever looks at the code in the future knows what's going on and how to work with said code.
src/main/java/io/github/debuggyteam/architecture_extensions/ArchitectureExtensions.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/debuggyteam/architecture_extensions/staticdata/StaticData.java
Show resolved
Hide resolved
Also regarding namespaces, it can potentially confuse someone if an arch-ex block made from "Example Mod" had |
This PR enables loader-agnostic mod integrations, but comes with some other changes:
This will take some careful documentation so people can get the most out of it, but in my opinion is a slam-dunk solution for #13