-
Notifications
You must be signed in to change notification settings - Fork 47
Add more missing function to ModList, FMLModContainer and ModContainer #87
Conversation
FMLModContainer and ModContainer: [API] Add getMod and acceptEvent methods [Internal] setParent and getParent: associated Fabric ModContainer getter and setters ModList: Add most helper functions present in Forge's ModList, these are frequenty used by mods ModLoader(class): A class may used by mods to post event on the FML event bus
TheGlitch76
left a comment
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 except for the getMod and setMod, which should be removed if they're just going to cause a mysterious NPE later.
patchwork-fml/src/main/java/net/minecraftforge/fml/javafmlmod/FMLModContainer.java
Show resolved
Hide resolved
|
|
||
| @Override | ||
| public Object getMod() { | ||
| return this.instance; |
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.
this seems to always be null, it might actually be better to drop them until the setter is actually called
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.
Should we just comment getMod() and setMod() for now? Impl setMod needs some changes in the patcher as well, the ForgeInitializer.onForgeInitialize should return the instance of the Forge mod class, so that we can record it in its ModContainer.
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.
yeah, just comment them out for now and the change that needs Patcher can be a separate PR
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.
Can we leave getMod() and setMod() here? Just add TODOs to both of them and throw a warning when they are called? Other methods in this pr require getMod(), simply remove getMod() needs more work.
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.
Alright. Make sure you open an issue on Patcher about it
TheGlitch76
left a comment
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'd like @coderbot16's review on this one because this part of the codebase is his territory, but looks good to me.
FMLModContainer and ModContainer:
[API] Add getMod and acceptEvent methods
[Internal] setParent and getParent: associated Fabric ModContainer
getter and setters
ModList:
Add most helper functions present in Forge's ModList, these are
frequenty used by mods
ModLoader(class):
A class may used by mods to post event on the FML event bus
TODO:
the ModContainer.setMod(Object) is not called anywhere in the code, I don't know how the forge mod main class instance is created. This is the only problem.