-
Notifications
You must be signed in to change notification settings - Fork 8
Fixes on hydraulic cannon #294
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
base: master
Are you sure you want to change the base?
Fixes on hydraulic cannon #294
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.
What's with all the changes in this class? It seems to be a lot more complex before but I cannot tell why it was needed. Is it not as simple as just checking the block and despawning the projectile if said block is not solid, every tick - or previous block as well or something if necessary?
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 have a task running uselessly if it can be stopped before
- Why currying up something that doesn't need currying and just makes a massive parameter functions when you can just have one constructor that handles it
- There are a lot of cases where the entity must be killed and the thing stopped, but intead are handled by a return, waiting on the removal task and kill task
So I rewrote the entire thing to match a similar logic to the confetti popper where we have a master task and everything dies when we are done, it is more like an optimization
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 currying up something that doesn't need currying and just makes a massive parameter functions when you can just have one constructor that handles it
Um... ????? The constructor has the same number of parameters as the 'massive parameter function' so this changes nothing except adding more code. There is no currying going on here either? Very confused here lol.
I see what you mean with the task not being cancelled, but as there are so few of these tasks running at once, and by far the most performance-intensive part is finding nearby entities/blocks and teleporting, it's a bit of a pointless optimisation.
If you do really want it as a class that is instantiated though, may I suggest you make it a PylonEntity instead? This is IMO a perfect use case for PylonEntity as we're wrapping an entity and adding our own logic. You could use the entity ticker stuff too.
src/main/java/io/github/pylonmc/pylon/base/util/DisplayProjectile.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Seggan <[email protected]>
Fixes #287 #278 #273