IForgeItem#46
Conversation
Tasks to resolve later: - `getToolTypes` and `getHarvestLevel` once `ToolType` is implemented - Use tag from Forge Tags class in `isBeaconPayment` once implemented - `initCapabilities` once `ICapabilityProvider` is added - `getAnimationParameters` once `ITimeValue` is added - Move contents of `getCreatorModId` to `ForgeHooksClient`?
coderbot16
left a comment
coderbot16
left a comment
There was a problem hiding this comment.
Most of the things in IForgeItem have not been truly implemented, because a lot of the functionality involves patches elsewhere that call these methods.
|
However, this is a good start! |
coderbot16
left a comment
coderbot16
left a comment
There was a problem hiding this comment.
This looks like a good start. I have some comment nitpicks. Also, it may be a good idea to split this into a patchwork-extensions-item module or similar because I can forsee just item extensions alone becoming huge.
coderbot16
left a comment
coderbot16
left a comment
There was a problem hiding this comment.
Looking much better, I see one or two minor logic issues but otherwise this looks ready. The comment about patchwork-extensions-item does still apply since the item mixins alone could become a few thousand lines of code, and it could be better to split it off. This should be an easy case of git mv if we do split it off. Thanks for all the hard work!
|
The CI failures are likely spurious, once you push some more code they should work. |
TheGlitch76
left a comment
TheGlitch76
left a comment
There was a problem hiding this comment.
Could a README.MD file be added to the root of the module that lists off things that need to be implemented elsewhere? I think that would make updating a lot easier than having to hunt things down
coderbot16
left a comment
coderbot16
left a comment
There was a problem hiding this comment.
Looks good, I'd just like the REACH_DISTANCE thing to be removed. Also, before I merge this, could you do the move to patchwork-extensions-item?
|
I think a README file is good, even if it is just a list of all the TODOs. |
coderbot16
left a comment
coderbot16
left a comment
There was a problem hiding this comment.
I'm curious, is patchwork-fml actually needed? Otherwise this looks good to merge.
|
Thanks for this! |
3 participants
Areas with tasks to be resolved in later PRs:
getToolTypesandgetHarvestLevelonceToolTypeis implementedisBeaconPaymentonce the class is addedinitCapabilitiesonceICapabilityProvideris addedgetAnimationParametersonceITimeValueis addedgetCreatorModIdtoForgeHooksClient?