Skip to content

Explosion Resistance API#7828

Closed
Astralchroma wants to merge 1 commit into
PaperMC:masterfrom
Astralchroma:explosion-resistance-api
Closed

Explosion Resistance API#7828
Astralchroma wants to merge 1 commit into
PaperMC:masterfrom
Astralchroma:explosion-resistance-api

Conversation

@Astralchroma
Copy link
Copy Markdown
Contributor

Adds API functions to Bukkit, Server, and Material to allow changing the explosion resistance of blocks without using NMS or Reflection which is the current method used to acomplish this.

As Material does not have a CraftBukkit implementation the functions to alter the resistance is part of Server, these functions are aliased in Bukkit and Material for easy access.

The existing getBlastResistance function has been deprecated as the name "explosion resistance" is used by Mojang Mappings. The function also previously used a switch statement to define the explosion resistance which has been commented out in favor of using the actually values.

A defaultExplosionResistance has been added to BlockBehaviour which stores the initial value, allowing a plugin to reset the explosion resistance to the value specified by vanilla minecraft.

@Astralchroma Astralchroma requested a review from a team as a code owner May 16, 2022 13:13
@Astralchroma
Copy link
Copy Markdown
Contributor Author

No idea why the tests are failing, the code works fine with runDev, may need some help figuring out the issue.

@Doc94
Copy link
Copy Markdown
Member

Doc94 commented May 16, 2022

No idea why the tests are failing, the code works fine with runDev, may need some help figuring out the issue.

the CI shows errors with test for testBlastResistance.

@Astralchroma
Copy link
Copy Markdown
Contributor Author

No idea why the tests are failing, the code works fine with runDev, may need some help figuring out the issue.

the CI shows errors with test for testBlastResistance.

I know that much, but the values it is returning is correct, so the tests should be passing, but they don't. I have never worked with tests before, so I don't really know how to resolve it.

@electronicboy
Copy link
Copy Markdown
Member

See the DummyServer class

@electronicboy
Copy link
Copy Markdown
Member

I think part of the reality is that those methods kinda shouldn't be on the Server itself, idk where they'd really be best suited though

@Astralchroma
Copy link
Copy Markdown
Contributor Author

Astralchroma commented May 16, 2022

I think part of the reality is that those methods kinda shouldn't be on the Server itself, idk where they'd really be best suited though

I agree, however putting them directly in the Material class is not really an option, and it didn't make sense to add a new singleton just for managing explosion resistance.

@Astralchroma
Copy link
Copy Markdown
Contributor Author

The test probably is not even needed anymore as all we are doing is comparing if the function is returning the very same variable we are comparing against.

@Astralchroma
Copy link
Copy Markdown
Contributor Author

Also what should be done about the switch statement I commented out, should I leave it commented out, or just remove it?

@Astralchroma
Copy link
Copy Markdown
Contributor Author

Rebased for 1.19

@Astralchroma
Copy link
Copy Markdown
Contributor Author

Fixed the issue with the test, was caused by DummyServer not having an implementation for getExplosionResistance().

Comment thread patches/server/0918-Explosion-Resistance-API.patch
@Astralchroma
Copy link
Copy Markdown
Contributor Author

Rebased

Copy link
Copy Markdown
Member

@electronicboy electronicboy left a comment

Choose a reason for hiding this comment

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

Generally looks fine, I'm somewhat iffy on methods being added to Bukkit for this, but, as said, there is no real general place for this stuff outside of Unsafe...

@Astralchroma
Copy link
Copy Markdown
Contributor Author

Rebased

@Astralchroma
Copy link
Copy Markdown
Contributor Author

Rebased for 1.19.1

@Astralchroma
Copy link
Copy Markdown
Contributor Author

Rebased

@Owen1212055
Copy link
Copy Markdown
Member

Thank you for your contribution, however, generally api like this is tricky as this is mutating data that really isn't expected to be mutated at all. Being able to modify a part of a block like this without notifying the client is a bit iffy. And in general, being able to change properties like this is something that we want to better support in the future. For now, it's better that these hard coded values stay immutable for the time being. If you want something like this it's most likely better to do this in your own fork.

@Astralchroma Astralchroma deleted the explosion-resistance-api branch March 30, 2023 19:21
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.

5 participants