Skip to content

Add ExplosiveMinecart ignite() and explode() overloads and add getIgniter() (#14097) - #14272

Open
WouterGritter wants to merge 2 commits into
PaperMC:mainfrom
WouterGritter:minecart-tnt-api-expansion
Open

WouterGritter wants to merge 2 commits into
PaperMC:mainfrom
WouterGritter:minecart-tnt-api-expansion

Conversation

@WouterGritter

@WouterGritter WouterGritter commented Sep 17, 2026

Copy link
Copy Markdown
Member

Adds:

  • ExplosiveMinecart#ignite(int fuseTime)
  • ExplosiveMinecart#ignite(Entity igniter)
  • ExplosiveMinecart#ignite(Entity igniter, int fuseTime)
  • ExplosiveMinecart#explode(Entity entity)
  • ExplosiveMinecart#explode(Entity entity, double power)
  • ExplosiveMinecart#getIgniter()

ignite(int fuseTime) was oddly missing - an earlier Paper patch added support for this through NMS but it wasn't exposed through the API before. ExplosiveMinecart#setFuseTicks exists "If the fuse ticks are set to a non-zero value, this will ignite the explosive." (from its javadocs), but this does not play the TNT_PRIMED sound and also doesn't fire EntityIgniteEvent. Should we deprecate this method in this PR?

This PR closes #14097 (Add .getIgniter() to ExplosiveMinecart).

At first it seemed better to expose MinecartTNT's DamageSource ignitionSource, but this is always an explosion damage source, optionally with the igniter entity as its cause, that the minecart will use once exploding.

MinecartTNT#primeFuse is a bit odd as it consumes a DamageSource source, which is the source that did damage to the minecart that caused it to prime, though the type is discarded. Right now I've got this wrapped in another explosion source, which gets stripped down to its causing entity, then re-wrapped in a new explosion source (by vanilla code).

@WouterGritter
WouterGritter requested a review from a team as a code owner September 17, 2026 13:09
@github-project-automation github-project-automation Bot moved this to Awaiting review in Paper PR Queue Sep 17, 2026
Comment thread paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftMinecartTNT.java Outdated

@Doc94 Doc94 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe i mess read something

import org.bukkit.entity.Entity;
import org.bukkit.entity.Explosive;
import org.bukkit.entity.Minecart;
import org.jetbrains.annotations.Nullable;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use JSpecify (and include the NullMarked)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@NullMarked is only used in the package-info.java's, and CommandMinecart also exists in this package with @NotNull / @Nullable annotations (jetbrains). Should I add @NullMarked to the package-info in this PR, and update CommandMinecart also? Seems a bit out of scope, but otherwise a @NullMarked in package-info would be misleading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The idea is avoid add the Jetbrains nullable annotation and move if is possible and not require big reworks (that is for another instance), you can by the moment just add to the Interface and later move to package when need.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Will do this for now, but this will be the first and only interface (or class) that is annotated by @NullMarked, the rest of the codebase only uses this in package-info.java

Comment thread paper-api/src/main/java/org/bukkit/entity/minecart/ExplosiveMinecart.java Outdated
@github-project-automation github-project-automation Bot moved this from Awaiting review to Changes required in Paper PR Queue Sep 17, 2026
@WouterGritter
WouterGritter force-pushed the minecart-tnt-api-expansion branch from 9b9d8e0 to 44f5442 Compare September 17, 2026 14:58
@WouterGritter

Copy link
Copy Markdown
Member Author

Force-pushed the requested changes (MinecartTNT now has no changes, apart from the AT), including a second commit that switches the org.bukkit.entity.minecart over to @NullMarked (only a minor change in CommandMinecart, out-of-scope-ish, let me know if this should be reversed)

@WouterGritter
WouterGritter force-pushed the minecart-tnt-api-expansion branch from aa6c2c5 to 5cdb6a5 Compare September 17, 2026 15:04
@Lulu13022002

Copy link
Copy Markdown
Contributor

This doesn't target the right branch should be on main (then depending when it's merged it can be moved to ver/26.2)

…me parameters, explode() overloads with the causing entity, and getIgniter() to obtain the entity that caused ignition
@WouterGritter
WouterGritter force-pushed the minecart-tnt-api-expansion branch from 5cdb6a5 to 0eeea53 Compare September 17, 2026 16:55
@WouterGritter
WouterGritter changed the base branch from dev/26.3 to main September 17, 2026 16:56
@WouterGritter

Copy link
Copy Markdown
Member Author

Rebased to main

@WouterGritter
WouterGritter requested a review from Doc94 September 18, 2026 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Changes required

Development

Successfully merging this pull request may close these issues.

4 participants