-
Notifications
You must be signed in to change notification settings - Fork 9
Feat powerful sponge #149
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?
Feat powerful sponge #149
Conversation
src/main/java/io/github/pylonmc/pylon/base/content/building/sponge/PowerfulSponge.java
Outdated
Show resolved
Hide resolved
Seggan
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.
You seem to have turned every range into getRange? Great job otherwise 👍
src/main/java/io/github/pylonmc/pylon/base/content/building/sponge/HotLavaSponge.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/pylonmc/pylon/base/content/building/sponge/PowerfulWaterSponge.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/pylonmc/pylon/base/content/building/sponge/PowerfulWaterSponge.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/pylonmc/pylon/base/content/building/sponge/PowerfulWaterSponge.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/pylonmc/pylon/base/content/building/sponge/WetWaterSponge.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Seggan <seggan21@gmail.com>
Co-authored-by: Seggan <seggan21@gmail.com>
Co-authored-by: Seggan <seggan21@gmail.com>
Co-authored-by: Seggan <seggan21@gmail.com>
src/main/java/io/github/pylonmc/pylon/base/content/building/sponge/WetWaterSponge.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/pylonmc/pylon/base/content/magic/FireproofRune.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/pylonmc/pylon/base/content/magic/FireproofRune.java
Outdated
Show resolved
Hide resolved
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.
Will need rebasing after the latest round of changes
Couple of issues I've found
- The lore of the sponges isn't in line with the other Pylon items. I'd recommend having a look at the lore of other items to see how we do things and try to make the sponges more in line with that. (For example, usage of colors, and the star icons have a specific use case)
- The new sponges drop vanilla sponges when broken?
- No recipe for hot lava sponge
I'll test other issues if InvUI no longer stop me from |
Seggan
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.
Needs updating to latest master/recipe system
src/main/java/io/github/pylonmc/pylon/base/content/building/sponge/HotLavaSponge.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/pylonmc/pylon/base/recipes/ImmerseRecipe.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/pylonmc/pylon/base/content/building/sponge/HotLavaSponge.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/pylonmc/pylon/base/content/building/sponge/PowerfulSponge.java
Show resolved
Hide resolved
src/main/java/io/github/pylonmc/pylon/base/content/building/sponge/HotLavaSponge.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/pylonmc/pylon/base/content/building/sponge/HotLavaSponge.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/pylonmc/pylon/base/content/building/sponge/WetPowerfulWaterSponge.java
Outdated
Show resolved
Hide resolved
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.
Am I correct in thinking this does not actually need to be split into multiple classes? Surely the behaviour of all the sponges can be unified into one class? I might be wrong though.
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.
For PowerfulWaterSponge and LavaSponge, yes.
But HotLavaSponge have more mechanism: REUSE_RATE and particles.
Also the WetPowerfulWaterSponge is a placeholder block. It's not a usable sponge in fact.
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.
Still have very mixed feelings about this turning either into obsidian or lava sponge. I would suggest we maybe just always turn it into lava sponge? any other thoughts?
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.
From my prospective, it is too powerful for HotLavaSponge. So turning it into obsidian is a way to limit it.
|
With regards to the ticking conversation, can't we just use SpongeAbsorbEvent via PylonSponge instead of ticking? I think the better approach here would be to remove the reuse rate mechanic and make the lava sponge reusable as I don't think it's that powerful considering it just deletes lava without giving it to you. If we're concerned about balance we could always lower the radius. This would allow all the sponge classes to be unified, thereby removing a lot of code and simplifying stuff, and would also be a nicer mechanic IMO. @Seggan @OhmV-IR @JustAHuman-xD do any of you have any opinions here? |
+1 agree |
|
About ticking, I have to use ticking if I need to absorb more than water, something like waterlogged block may won't trigger the event. |
I don't really care honestly |
…eat-powerful-sponge
|
Added |



Closes #41