-
Notifications
You must be signed in to change notification settings - Fork 103
Display entity projectiles #1502
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: dev
Are you sure you want to change the base?
Changes from 27 commits
e4d849b
1a0accf
f26e050
8254291
e96d71f
9b72e57
3675f0b
978d6fd
f9f70df
5d966ca
6e69021
4352ed9
a8fa373
e2a746b
b6fb019
9763acf
00706de
5688d86
9fe84da
20a889a
73be187
47e03dd
7e78085
34cc4d6
10c0c0d
4497d8f
00282c6
f98dd14
79e53b6
b8e5d91
3fd6a6d
5a9698a
b61b00b
25afbbc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -15,13 +15,16 @@ public class ProjectileDefinition extends SelfIdentifyingFeatureDefinition { | |||||||||||||||||||||||||||||||||||
| protected @Nullable Float power; | ||||||||||||||||||||||||||||||||||||
| protected double velocity; | ||||||||||||||||||||||||||||||||||||
| protected ClickAction clickAction; | ||||||||||||||||||||||||||||||||||||
| protected Class<? extends Entity> projectile; | ||||||||||||||||||||||||||||||||||||
| protected ProjectileEntity projectile; | ||||||||||||||||||||||||||||||||||||
| protected List<PotionEffect> potion; | ||||||||||||||||||||||||||||||||||||
| protected Filter destroyFilter; | ||||||||||||||||||||||||||||||||||||
| protected Duration coolDown; | ||||||||||||||||||||||||||||||||||||
| protected boolean throwable; | ||||||||||||||||||||||||||||||||||||
| protected boolean precise; | ||||||||||||||||||||||||||||||||||||
| protected BlockMaterialData blockMaterial; | ||||||||||||||||||||||||||||||||||||
| protected float scale; | ||||||||||||||||||||||||||||||||||||
| protected boolean solidBlockCollision; | ||||||||||||||||||||||||||||||||||||
| protected Duration maxTravelTime; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| public ProjectileDefinition( | ||||||||||||||||||||||||||||||||||||
| @Nullable String id, | ||||||||||||||||||||||||||||||||||||
|
|
@@ -30,13 +33,16 @@ public ProjectileDefinition( | |||||||||||||||||||||||||||||||||||
| @Nullable Float power, | ||||||||||||||||||||||||||||||||||||
| double velocity, | ||||||||||||||||||||||||||||||||||||
| ClickAction clickAction, | ||||||||||||||||||||||||||||||||||||
| Class<? extends Entity> entity, | ||||||||||||||||||||||||||||||||||||
| ProjectileEntity entity, | ||||||||||||||||||||||||||||||||||||
| List<PotionEffect> potion, | ||||||||||||||||||||||||||||||||||||
| Filter destroyFilter, | ||||||||||||||||||||||||||||||||||||
| Duration coolDown, | ||||||||||||||||||||||||||||||||||||
| boolean throwable, | ||||||||||||||||||||||||||||||||||||
| boolean precise, | ||||||||||||||||||||||||||||||||||||
| BlockMaterialData blockMaterial) { | ||||||||||||||||||||||||||||||||||||
| BlockMaterialData blockMaterial, | ||||||||||||||||||||||||||||||||||||
| float scale, | ||||||||||||||||||||||||||||||||||||
| boolean solidBlockCollision, | ||||||||||||||||||||||||||||||||||||
| Duration maxTravelTime) { | ||||||||||||||||||||||||||||||||||||
| super(id); | ||||||||||||||||||||||||||||||||||||
| this.name = name; | ||||||||||||||||||||||||||||||||||||
| this.damage = damage; | ||||||||||||||||||||||||||||||||||||
|
|
@@ -50,6 +56,31 @@ public ProjectileDefinition( | |||||||||||||||||||||||||||||||||||
| this.throwable = throwable; | ||||||||||||||||||||||||||||||||||||
| this.precise = precise; | ||||||||||||||||||||||||||||||||||||
| this.blockMaterial = blockMaterial; | ||||||||||||||||||||||||||||||||||||
| this.scale = scale; | ||||||||||||||||||||||||||||||||||||
| this.solidBlockCollision = solidBlockCollision; | ||||||||||||||||||||||||||||||||||||
| this.maxTravelTime = maxTravelTime; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| public static sealed class ProjectileEntity { | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
| public static sealed class ProjectileEntity { | |
| public sealed interface ProjectileEntity permits RealEntity, CustomEntity { |
Outdated
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.
Abstract has a very specific meaning in the java world, and given this class isn't even abstract, i'd vote against it, and instead probably use something like CustomEntity
Outdated
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 can probably use a record for these:
| public static final class RealEntity extends ProjectileEntity { | |
| public final Class<? extends Entity> entityType; | |
| public RealEntity(Class<? extends Entity> entityType) { | |
| this.entityType = entityType; | |
| } | |
| } | |
| public static final class AbstractEntity extends ProjectileEntity { | |
| public final AbstractEntityType entityType; | |
| public AbstractEntity(AbstractEntityType entityType) { | |
| this.entityType = entityType; | |
| } | |
| } | |
| record RealEntity(Class<? extends Entity> entityType) implements ProjectileEntity {} | |
| record CustomEntity(CustomEntityType type) implements ProjectileEntity {} |
Outdated
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.
Same as above, avoid Abstract when something is not abstract
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,10 +1,17 @@ | ||||||||||||||||||||
| package tc.oc.pgm.projectile; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import static tc.oc.pgm.util.Assert.assertTrue; | ||||||||||||||||||||
| import static tc.oc.pgm.util.nms.Packets.ENTITIES; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import com.google.common.collect.ImmutableSet; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import java.util.Collection; | ||||||||||||||||||||
| import java.util.HashMap; | ||||||||||||||||||||
| import java.util.UUID; | ||||||||||||||||||||
| import java.util.concurrent.*; | ||||||||||||||||||||
|
||||||||||||||||||||
| import java.util.function.BooleanSupplier; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import org.bukkit.Location; | ||||||||||||||||||||
| import org.bukkit.Material; | ||||||||||||||||||||
| import org.bukkit.block.Block; | ||||||||||||||||||||
| import org.bukkit.entity.Entity; | ||||||||||||||||||||
|
|
@@ -37,16 +44,22 @@ | |||||||||||||||||||
| import tc.oc.pgm.api.match.Match; | ||||||||||||||||||||
| import tc.oc.pgm.api.match.MatchModule; | ||||||||||||||||||||
| import tc.oc.pgm.api.match.MatchScope; | ||||||||||||||||||||
| import tc.oc.pgm.api.party.Party; | ||||||||||||||||||||
| import tc.oc.pgm.api.player.MatchPlayer; | ||||||||||||||||||||
| import tc.oc.pgm.api.player.ParticipantState; | ||||||||||||||||||||
| import tc.oc.pgm.events.ListenerScope; | ||||||||||||||||||||
| import tc.oc.pgm.events.PlayerParticipationStopEvent; | ||||||||||||||||||||
| import tc.oc.pgm.filters.query.BlockQuery; | ||||||||||||||||||||
| import tc.oc.pgm.filters.query.PlayerBlockQuery; | ||||||||||||||||||||
| import tc.oc.pgm.kits.tag.ItemTags; | ||||||||||||||||||||
| import tc.oc.pgm.projectile.path.LinearProjectilePath; | ||||||||||||||||||||
| import tc.oc.pgm.projectile.path.ProjectilePath; | ||||||||||||||||||||
| import tc.oc.pgm.util.bukkit.MetadataUtils; | ||||||||||||||||||||
| import tc.oc.pgm.util.inventory.InventoryUtils; | ||||||||||||||||||||
| import tc.oc.pgm.util.nms.NMSHacks; | ||||||||||||||||||||
| import tc.oc.pgm.util.nms.packets.BlockEntity; | ||||||||||||||||||||
| import tc.oc.pgm.util.nms.packets.FakeEntity; | ||||||||||||||||||||
| import tc.oc.pgm.util.nms.packets.Packet; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @ListenerScope(MatchScope.RUNNING) | ||||||||||||||||||||
| public class ProjectileMatchModule implements MatchModule, Listener { | ||||||||||||||||||||
|
|
@@ -88,32 +101,96 @@ && isValidProjectileAction(event.getAction(), projectileDefinition.clickAction)) | |||||||||||||||||||
|
|
||||||||||||||||||||
| if (this.isCooldownActive(player, projectileDefinition)) return; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| boolean realProjectile = Projectile.class.isAssignableFrom(projectileDefinition.projectile); | ||||||||||||||||||||
| boolean realProjectile = false; | ||||||||||||||||||||
| if (projectileDefinition.projectile instanceof ProjectileDefinition.ProjectileEntity.RealEntity) { | ||||||||||||||||||||
| realProjectile = Projectile.class.isAssignableFrom( | ||||||||||||||||||||
| ((ProjectileDefinition.ProjectileEntity.RealEntity) projectileDefinition.projectile).entityType | ||||||||||||||||||||
| ); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
||||||||||||||||||||
| boolean realProjectile = false; | |
| if (projectileDefinition.projectile instanceof ProjectileDefinition.ProjectileEntity.RealEntity) { | |
| realProjectile = Projectile.class.isAssignableFrom( | |
| ((ProjectileDefinition.ProjectileEntity.RealEntity) projectileDefinition.projectile).entityType | |
| ); | |
| } | |
| var projType = projectileDefinition.projectile; | |
| boolean realProjectile = projType instanceof ProjectileDefinition.ProjectileEntity.RealEntity re | |
| && Projectile.class.isAssignableFrom(re.entityType()); |
Let's make use of java 17 features too
Outdated
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.
this is very ugly, and i'm starting to believe now that we have wrapper entities we can probably hide away these differences inside such that you can:
projectileDefinition.projectile.launch(player, velocity) and this logic is within there, not just for this but for the others too
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.
I was attempting to do this, but it was taking some time and effort to figure out. In the end, I left it and I would rather potentially come back to it later than to do it as part of this PR. I think it is ok to have here for now.
Outdated
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.
this seems like logic that the version-dependant impl should handle, if it needs it
Outdated
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.
| } else if (projectileDefinition.projectile instanceof ProjectileDefinition.ProjectileEntity.RealEntity) { | |
| projectile = | |
| player.getWorld().spawn(loc, ((ProjectileDefinition.ProjectileEntity.RealEntity) projectileDefinition.projectile).entityType); | |
| } | |
| } else if (projectileDefinition.projectile instanceof ProjectileDefinition.ProjectileEntity.RealEntity re) { | |
| projectile = player.getWorld().spawn(loc, re.entityType()); | |
| } |
really should use java 17's features in a bunch of these
Outdated
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.
this is a nope, you're running version-dependant logic in the common area, this is something the wrapper for your entity should handle
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.
Same as what I mentioned in the other comment, I am hoping we can let this go for now and perhaps come back to add the wrapper later. I was having trouble finding a nice way to implement it to properly account for all scenarios, perhaps if you know a good way then you could add it quickly.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,11 +1,13 @@ | ||||||||||||||||||||||||
| package tc.oc.pgm.projectile; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| import com.google.common.collect.ImmutableList; | ||||||||||||||||||||||||
| import com.google.common.collect.ImmutableMap; | ||||||||||||||||||||||||
| import com.google.common.collect.ImmutableSet; | ||||||||||||||||||||||||
| import java.time.Duration; | ||||||||||||||||||||||||
| import java.util.Collection; | ||||||||||||||||||||||||
| import java.util.HashSet; | ||||||||||||||||||||||||
| import java.util.List; | ||||||||||||||||||||||||
| import java.util.Map; | ||||||||||||||||||||||||
| import java.util.Set; | ||||||||||||||||||||||||
| import java.util.logging.Logger; | ||||||||||||||||||||||||
| import org.bukkit.entity.Arrow; | ||||||||||||||||||||||||
|
|
@@ -23,6 +25,7 @@ | |||||||||||||||||||||||
| import tc.oc.pgm.filters.parse.FilterParser; | ||||||||||||||||||||||||
| import tc.oc.pgm.kits.KitParser; | ||||||||||||||||||||||||
| import tc.oc.pgm.util.material.BlockMaterialData; | ||||||||||||||||||||||||
| import tc.oc.pgm.util.nms.NMSHacks; | ||||||||||||||||||||||||
| import tc.oc.pgm.util.xml.InvalidXMLException; | ||||||||||||||||||||||||
| import tc.oc.pgm.util.xml.Node; | ||||||||||||||||||||||||
| import tc.oc.pgm.util.xml.XMLUtils; | ||||||||||||||||||||||||
|
|
@@ -62,9 +65,9 @@ public ProjectileModule parse(MapFactory factory, Logger logger, Document doc) | |||||||||||||||||||||||
| Node.fromChildOrAttr(projectileElement, "velocity"), Double.class, 1.0); | ||||||||||||||||||||||||
| ClickAction clickAction = XMLUtils.parseEnum( | ||||||||||||||||||||||||
| Node.fromAttr(projectileElement, "click"), ClickAction.class, ClickAction.BOTH); | ||||||||||||||||||||||||
| Class<? extends Entity> entity = | ||||||||||||||||||||||||
| XMLUtils.parseEntityTypeAttribute(projectileElement, "projectile", Arrow.class); | ||||||||||||||||||||||||
| BlockMaterialData blockMaterial = entity.isAssignableFrom(FallingBlock.class) | ||||||||||||||||||||||||
| ProjectileDefinition.ProjectileEntity entity = | ||||||||||||||||||||||||
| parseProjectileEntity(projectileElement, "projectile", Arrow.class); | ||||||||||||||||||||||||
| BlockMaterialData blockMaterial = acceptsBlockMaterial(entity) | ||||||||||||||||||||||||
| ? XMLUtils.parseBlockMaterialData(Node.fromAttr(projectileElement, "material")) | ||||||||||||||||||||||||
| : null; | ||||||||||||||||||||||||
| Float power = XMLUtils.parseNumber( | ||||||||||||||||||||||||
|
|
@@ -76,6 +79,9 @@ public ProjectileModule parse(MapFactory factory, Logger logger, Document doc) | |||||||||||||||||||||||
| boolean throwable = | ||||||||||||||||||||||||
| XMLUtils.parseBoolean(projectileElement.getAttribute("throwable"), true); | ||||||||||||||||||||||||
| boolean precise = XMLUtils.parseBoolean(projectileElement.getAttribute("precise"), true); | ||||||||||||||||||||||||
| float scale = XMLUtils.parseNumber(Node.fromChildOrAttr(projectileElement, "scale"), Float.class, 1.0f); | ||||||||||||||||||||||||
| boolean solidBlockCollision = XMLUtils.parseBoolean(projectileElement.getAttribute("solid-block-collision"), true); | ||||||||||||||||||||||||
| Duration maxTravelTime = XMLUtils.parseDuration(projectileElement.getAttribute("max-travel-time"), Duration.ofSeconds(1)); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ProjectileDefinition projectileDefinition = new ProjectileDefinition( | ||||||||||||||||||||||||
| id, | ||||||||||||||||||||||||
|
|
@@ -90,13 +96,53 @@ public ProjectileModule parse(MapFactory factory, Logger logger, Document doc) | |||||||||||||||||||||||
| coolDown, | ||||||||||||||||||||||||
| throwable, | ||||||||||||||||||||||||
| precise, | ||||||||||||||||||||||||
| blockMaterial); | ||||||||||||||||||||||||
| blockMaterial, | ||||||||||||||||||||||||
| scale, | ||||||||||||||||||||||||
| solidBlockCollision, | ||||||||||||||||||||||||
| maxTravelTime); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| factory.getFeatures().addFeature(projectileElement, projectileDefinition); | ||||||||||||||||||||||||
| projectiles.add(projectileDefinition); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| return projectiles.isEmpty() ? null : new ProjectileModule(ImmutableSet.copyOf(projectiles)); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| private static final Map<String, ProjectileDefinition.ProjectileEntity.AbstractEntity> ABSTRACT_ENTITY_MAP = | ||||||||||||||||||||||||
| ImmutableMap.of( | ||||||||||||||||||||||||
| "block", new ProjectileDefinition.ProjectileEntity.AbstractEntity( | ||||||||||||||||||||||||
| ProjectileDefinition.ProjectileEntity.AbstractEntityType.BLOCK | ||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| private static ProjectileDefinition.ProjectileEntity parseProjectileEntity( | ||||||||||||||||||||||||
| final Element element, | ||||||||||||||||||||||||
| final String attributeName, | ||||||||||||||||||||||||
| final Class<? extends Entity> def | ||||||||||||||||||||||||
| ) throws InvalidXMLException { | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
| private static ProjectileDefinition.ProjectileEntity parseProjectileEntity( | |
| final Element element, | |
| final String attributeName, | |
| final Class<? extends Entity> def | |
| ) throws InvalidXMLException { | |
| private static ProjectileDefinition.ProjectileEntity parseProjectileEntity(XmlFluentParser parser, Element el) throws InvalidXMLException { |
Attribute name is always "projectile" and default entity is always arrow, there's no need to make them parameters, instead pass in the parser which will be of use
Outdated
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.
i don't see the need for this, if nothing matches it you'll find out anyways
Outdated
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.
Given custom logic will likely have to be involved, consider just using a switch, and using the fluent parser:
| if (ABSTRACT_ENTITY_MAP.containsKey(entityText.toLowerCase())) { | |
| return ABSTRACT_ENTITY_MAP.get(entityText.toLowerCase()); | |
| } | |
| return new ProjectileDefinition.ProjectileEntity.RealEntity(XMLUtils.parseEntityTypeAttribute(element, attributeName, def)); | |
| return switch(entityText.toLowerCase(Locale.ROOT)) { | |
| case "entity" -> new ProjectileDefinition.BlockEntity( | |
| parser.parseFloat(el, "scale").optional(1.0f), | |
| parser.parseBool(el, "solid-block-collision").orTrue(), | |
| parser.parseDuration(el, "max-travel-time").optional(DEFAULT_MAX_TRAVEL); | |
| default -> new ProjectileDefinition.RealEntity(XMLUtils.parseEntityTypeAttribute(node)); | |
| } |
Outdated
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.
This seems like logic that ProjectileEntity should implement instead
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.
These seem to only affect custom projectile entities, so it probably makes sense that they'd go in as part of the
CustomEntityimplementation of ProjectileEntity.Additionally
scaleshould probably besizeas that leaves it more open to future projectiles (ie: raytraced beam)