-
Notifications
You must be signed in to change notification settings - Fork 9
[ref:idras-little-fixes] Idras little fixes #497
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
Conversation
...n/java/io/github/pylonmc/pylon/base/content/machines/hydraulics/HydraulicCoreDrillHatch.java
Show resolved
Hide resolved
| private double rotation; | ||
| @Setter | ||
| private boolean locked; | ||
| public boolean locked; |
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 has this been made public?
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.
Might be useful to be able to get the value from outside of the pedestal
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 not the getter/setter then
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.
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.
- Consistency
- Best practices
- What else do we use lombok for?
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.
Are we really going to argue about this? Does it really matter? Does it affect anything? Most fields across base are just public in case someone wants to access something for some unknown reason.
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.
Like I'll change it if you insist but it just feels a little like you picked the smallest, most insignificant possible thing from this PR to comment on XD
src/main/java/io/github/pylonmc/pylon/base/content/tools/HydraulicCannon.java
Outdated
Show resolved
Hide resolved
|
I've taken the liberty to de-delta the smeltery in my own PR |
Do you want me to remove the changes I made here then? |
|
Yeah I've changed it more comprehensively |
|
Ok I'm gonna have to keep the changes since I've removed deltaSeconds entirely in core, so this won't compile in that case. Is it OK if we keep them, and then you merge this PR (I'm assuming this PR will be merged into master before yours) into your PR, or just override my changes completely? |
|
Yeyeye ofc |
Just a ton of small things both codewise and in-game, there were too many to really split it up into many PRs without completely losing my sanity (sorry). Here is the list of changes. Half of this typed out on 1 hour sleep sorry if no makey sense
ItemStackBuilderto remove boilerplateTested pretty much all blocks (multiple times) but have likely introduced plenty of bugs due to the scope of the changes :(
Pretty much ignored smeltery here, still can't conjure the courage to go and fight that particular boss battle