Skip to content
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

Sponge & Explosion fixes by Cody #103

Merged
merged 2 commits into from
Aug 18, 2024
Merged

Conversation

moderatorman
Copy link
Collaborator

These changes were written and provided by Cody. All credit to him.
I've tested the changes and personally vouch for them.

@moderatorman moderatorman requested review from RhysB and removed request for RhysB August 9, 2024 22:46
@RhysB
Copy link
Owner

RhysB commented Aug 12, 2024

Looks good. just a small suggestion—could we add some info for fix.optimize-sponges? Maybe something like:

fix.optimize-sponges.enabled: true
fix.optimize-sponges.info: Optimizes sponges by doing ABC

I have been trying to do that for any new settings added to Poseidon so at least the config has some documentation.

Copy link
Contributor

@notdevcody notdevcody left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix.optimize-sponges -> fix.optimize-sponges.enabled

@moderatorman
Copy link
Collaborator Author

fix.optimize-sponges -> fix.optimize-sponges.enabled

Look, the code works so I'm leaving it as is. If the muffin man wants it to be a certain way, he can merge it and change it himself. The code is offered as is.

@moderatorman
Copy link
Collaborator Author

moderatorman commented Aug 14, 2024

Coming up on a week now @RhysB. I added the info thing to the config.
But if the .enabled thing is really that important to you, you're gonna have to add it yourself. I have way better things to do. I'm sure you do too, which is why it only takes one click for you to merge this. I don't mean to be an ass, but if you want the config changed, you're gonna have to do it yourself once this is merged. I have my own server to run, and I was doing this as a favor to code because I thought I would be able to merge it myself, which I can't, so now I've just wasted my time basically. If code submitted this himself, I would've just merged it as is instantly because there's a grand total of like 4 people that use this thing and something tells me none of them will care about the config.

@RhysB RhysB merged commit 5420c2d into RhysB:master Aug 18, 2024
2 checks passed
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.

3 participants