Skip to content

Made an Accessor Mixin for getting attachments straight from PhysShip.#1

Open
NythicalNorm wants to merge 1 commit intoblockninja124:1.20.1/mainfrom
NythicalNorm:1.20.1/main
Open

Made an Accessor Mixin for getting attachments straight from PhysShip.#1
NythicalNorm wants to merge 1 commit intoblockninja124:1.20.1/mainfrom
NythicalNorm:1.20.1/main

Conversation

@NythicalNorm
Copy link

Brickyboy showed me the ConcurrentModificationException try catch, I thought I saw that there were references to the attachments stored in PhysShip, so tried to make it work with an Accessor Mixin, so now no need for the sus try catch, right now the belts are not working on the main branch (also MixinBeltBlockEntity has some compile errors), I tested on a old commit and saw that my code does work. if you comment out the errors in MixinBeltBlockEntity, you can see that my code does get the BeltStorageForceInducer attachment from the PhysShip (you could also see that with a breakpoint).

This implementation may be slower than the original implementation if there are a lot of loaded ships. due to this needing to iterate through every attachment on the PhyShip, also I don't know how future-proof this is against changes in VS since this is explicitly not using the API and bypassing it kinda.

An optimization that could be made is caching which ships have the belt attachment on the server ship load event (and at the code where you add the attachment) into a hashmap with the ship id as key, and checking if a ship id is in the hashmap before executing the iteration through all the attachments in the ship, still this should be okay as is.

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.

1 participant