-
-
Notifications
You must be signed in to change notification settings - Fork 108
Added PlayerNameEntityScriptEvent #2708
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?
Conversation
// | ||
// @Cancellable true | ||
// | ||
// @Triggers when a player is attempting to rename an entity. |
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.
the words name tag
should appear somewhere in this meta. generally anything somebody might try to search for when they want this event, should be written somewhere in it
// <context.persistent> returns whether this will cause the entity to persist through server restarts. | ||
// | ||
// @Determine | ||
// "NAME:ElementTag" to set a different name for the entity. |
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.
"NAME:<ElementTag>"
// | ||
// @Context | ||
// <context.entity> returns an EntityTag of the renamed entity. | ||
// <context.old_name> returns the old name of the entity. |
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.
what value does this hold for an entity that wasn't named previously?
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.
the entity type
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.
To be clear: the answer to that question should be written in the meta
// @Determine | ||
// "NAME:ElementTag" to set a different name for the entity. | ||
// "PERSISTENT" to indicate that the entity should remain persistent. | ||
// "NOT_PERSISTENT" to indicate that the entity should not remain persistent. |
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.
Does this cause the entity to not remain persistent, or does this stop the event from modifying the persistence state?
return switch (name) { | ||
case "entity" -> entity.getDenizenObject(); | ||
case "name" -> new ElementTag(PaperModule.stringifyComponent(event.getName())); | ||
case "old_name" -> entity.getName().equals(entity.getEntityType().toString()) ? null : new ElementTag(entity.getName()); |
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 check logic is a very wonky hack
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.
... also this isn't even event data / from the event at all, why is this here?
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 think that with this event, having both what the name is being changed to and what it was would be beneficial. With the wonkiness, would you prefer that it just returned the entity type if there was no old name, because that's what it outputs in that scenario.
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'd need to read the old name in the event block, not the getContext, to preserve it properly for after
events, and you'd need to use the correct method to get the name (look into the custom_name
tag) not that getName method.
|
||
public PlayerNameEntityEvent event; | ||
public EntityTag entity; | ||
public String old_name; |
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.
oldName
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.
also just make it an ElementTag here
public void playerNamesEntity(PlayerNameEntityEvent event) { | ||
this.event = event; | ||
entity = new EntityTag(event.getEntity()); | ||
oldName = PaperAPITools.instance.getCustomName(entity.getBukkitEntity()) == null ? null : new ElementTag(PaperAPITools.instance.getCustomName(entity.getBukkitEntity())); |
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.
don't dup that entire section of code
// | ||
// @Determine | ||
// "NAME:<ElementTag>" to set a different name for the entity. | ||
// "PERSISTENT:<ElementTag(Boolean)>" to set whether the entity should remain through server restarts. |
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.
Repeating my earlier question: Does false
on this cause the entity to not remain persistent, or does this stop the event from modifying the persistence state?
// | ||
// @Determine | ||
// "NAME:<ElementTag>" to set a different name for the entity. | ||
// "PERSISTENT:<ElementTag(Boolean)>" to set whether the entity should remain through server restarts. ("True" keeps the entity across server restarts, "false" gets rid of the entity when the server restarts.) |
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.
The way this was edited does not directly address my question: Does false
on this cause the entity to not remain persistent, or does this stop the event from modifying the persistence state?
// | ||
// @Determine | ||
// "NAME:<ElementTag>" to set a different name for the entity. | ||
// "NOT_PERSISTENT" to override the default behavior of named entities persisting through server restarts. |
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're continuing to randomly modify this code without answering my question
Closing this PR until you respond to the question |
The persistence state will not be modified for every event involving that entity following the first case where the |
... what. |
this.<PlayerNameEntityScriptEvent, ElementTag>registerDetermination("persistent", ElementTag.class, (evt, context, determination) -> { | ||
event.setPersistent(determination.asBoolean()); | ||
}); |
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 should probably be registerOptionalDetermination
and return false
and/or error if the element isn't a boolean?
registerCouldMatcher("player names <entity>"); | ||
this.<PlayerNameEntityScriptEvent, ElementTag>registerOptionalDetermination("persistent", ElementTag.class, (evt, context, determination) -> { | ||
if (determination.isBoolean()) { | ||
event.setPersistent(determination.asBoolean()); |
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.
Need to access it from the evt
parameter
return false; | ||
}); | ||
this.<PlayerNameEntityScriptEvent, ElementTag>registerDetermination("name", ElementTag.class, (evt, context, determination) -> { | ||
event.setName(PaperModule.parseFormattedText(determination.toString(), ChatColor.WHITE)); |
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 here
public ObjectTag getContext(String name) { | ||
return switch (name) { | ||
case "entity" -> entity.getDenizenObject(); | ||
case "name" -> new ElementTag(PaperModule.stringifyComponent(event.getName())); |
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.
Nitpick, but this can be a plain text element (, true
)
this.event = event; | ||
entity = new EntityTag(event.getEntity()); | ||
String name = PaperAPITools.instance.getCustomName(entity.getBukkitEntity()); | ||
oldName = name == null ? null : new ElementTag(name); |
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 here
Requested in https://discord.com/channels/315163488085475337/1346666007049015398, triggers when a player uses a name tag on an entity and changes its name