Bridge egg#1686
Conversation
Signed-off-by: Eliminate123 <96848330+eliminate123@users.noreply.github.com>
|
Oh golly do I already have a map that could use this 😈 |
| boolean bridgeEgg = | ||
| XMLUtils.parseBoolean(projectileElement.getAttribute("bridge-egg"), false); |
There was a problem hiding this comment.
instead of bridge-egg="true" i'd expect projectile="bridge-egg", which goes in line with what i mentioned about supporting other projectile types that aren't vanilla entities but rather more expansive concepts
There was a problem hiding this comment.
I made it so it now uses projectile="bridge-egg"
| if (bridgeRange < MIN_BRIDGE_RANGE) { | ||
| throw new InvalidXMLException( | ||
| "'bridge-range' must be at least " + MIN_BRIDGE_RANGE, projectileElement); | ||
| } | ||
| if (bridgeRange > MAX_BRIDGE_RANGE) { | ||
| bridgeRange = MAX_BRIDGE_RANGE; | ||
| } | ||
|
|
||
| if (bridgeMaterials.isEmpty()) { | ||
| throw new InvalidXMLException( | ||
| "'bridge-material' has not been defined", projectileElement); | ||
| } | ||
| if (power != null) { | ||
| throw new InvalidXMLException( | ||
| "'bridge-egg' and 'power' cannot both be defined", projectileElement); | ||
| } | ||
| if (potionKit != null && !potionKit.isEmpty()) { | ||
| throw new InvalidXMLException( | ||
| "'bridge-egg' and 'effect' cannot both be defined", projectileElement); | ||
| } | ||
| if (destroyFilter != null) { | ||
| throw new InvalidXMLException( | ||
| "'bridge-egg' and 'destroy-filter' cannot both be defined", projectileElement); | ||
| } | ||
| } else { | ||
| if (bridgeRange != 0) { | ||
| throw new InvalidXMLException( | ||
| "'bridge-range' is only supported for 'bridge-egg'", projectileElement); | ||
| } | ||
| if (!bridgeMaterials.isEmpty()) { | ||
| throw new InvalidXMLException( | ||
| "'bridge-material' is only supported for 'bridge-egg'", projectileElement); | ||
| } |
There was a problem hiding this comment.
This is pain copypaste slop of mutually exclusive tags, many of them aren't even necessarily mutually exclusive they're just unused depending on what is being parsed. So bridge eggs just shouldn't be reading stuff like power, and non-bridge eggs shouldn't be reading stuff like bridge-range.
PGM will report them as unused nodes and that's all you really need here.
Signed-off-by: Eliminate123 <96848330+eliminate123@users.noreply.github.com>
| protected Class<? extends Entity> projectile; | ||
| protected List<PotionEffect> potion; | ||
| protected Filter destroyFilter; | ||
| protected Duration coolDown; | ||
| protected boolean throwable; | ||
| protected boolean precise; | ||
| protected BlockMaterialData blockMaterial; | ||
| protected final int bridgeRange; | ||
| protected List<Material> bridgeMaterial; | ||
| protected boolean teamColor; | ||
| protected boolean silent; |
There was a problem hiding this comment.
Maybe i didn't make a clear enough comment in the first review, but what i mean is that the definition shouldn't have stuff not relevant to it, so i'd expect this to instead work more like:
| protected boolean silent; | |
| protected PgmProjectile projectile; | |
| protected List<PotionEffect> potion; | |
| protected Filter destroyFilter; | |
| protected Duration coolDown; | |
| protected boolean throwable; | |
| protected boolean precise; | |
| protected BlockMaterialData blockMaterial; |
where that PgmProjectile can be a direct entity can be something along the lines of:
interface PgmProjectile {
public void spawn(...);
public void despawn(...);
}and you can have differing implementations like:
record EntityProjectile(Class<? extends Entity> projectile) implements PgmProjectile {...} (current behavior)
record BridgeEggProjectile(int range, List<Material> material, boolean teamColor, boolean silent) implements PgmProjectile { ... }
That means that attributes that are specific to one type are only parsed by that type, so in the parsing code you can have something like:
PgmProjectile projectile = switch (projType) {
case "bridge-egg" -> new BridgeEggProjectile(
parser.parseInt(el, "bridge-range"),
parser.parseMaterial(...),
parser.parseBool(el, "bridge-team-color"),
parser.parseBool(el, "bridge-silent"));
default -> new EntityProjectile(...);
}
This way the attributes that are specific to bridge-eggs only get parsed if bridge egg is relevant, and are passed along with it.
You can see a big portion of this same idea implemented in #1502 (https://github.com/PGMDev/PGM/pull/1502/changes#diff-079be93d9cad6596ef0f505dad1789ed6b2f93ac49776622e21b4e82f3df6ae5R19)
This PR adds new attributes for
<projectile>:bridge-egg="true/false"bridge-range="integer"(Distance bridge continues before stopping)bridge-material="filter"(Blocks used for bridge. If >1 block type, randomizes placement)team-color="true/false"(Dyes dyeable blocks)silent="true/false"(Mutes bridge noises)If a player leaves/obs/switches teams with an active bridge egg, its killed. If a bridge egg hits any region where building isnt enabled the bridge egg is killed. If the bridge egg hits vanilla height limit/ it gets killed. If a bridge egg hits the void it gets killed.