Add Support for Scoreboard Teams on Folia#3296
Add Support for Scoreboard Teams on Folia#3296Jsinco wants to merge 11 commits intoCitizensDev:masterfrom
Conversation
| CitizensAPI.getScheduler().runEntityTask(npc.getEntity(), () -> { | ||
| boolean despawned = npc.despawn(DespawnReason.WORLD_UNLOAD); | ||
| if (event.isCancelled() || !despawned) { | ||
| for (ChunkCoord coord : Lists.newArrayList(toRespawn.keySet())) { | ||
| if (event.getWorld().getUID().equals(coord.worldUUID)) { | ||
| respawnAllFromCoord(coord, event); | ||
| } | ||
| } | ||
| event.setCancelled(true); | ||
| return; | ||
| } | ||
| event.setCancelled(true); | ||
| return; | ||
| } | ||
| if (npc.isSpawned()) { | ||
| toRespawn.put(new ChunkCoord(npc.getEntity().getLocation()), npc); | ||
| Messaging.debug("Despawned", npc, "due to world unload at", event.getWorld().getName()); | ||
| } | ||
| if (npc.isSpawned()) { | ||
| toRespawn.put(new ChunkCoord(npc.getEntity().getLocation()), npc); | ||
| Messaging.debug("Despawned", npc, "due to world unload at", event.getWorld().getName()); | ||
| } | ||
| }); |
There was a problem hiding this comment.
The event cannot be canceled because you are delayed to the next tick, and therefore the event will already have been resolved.
There was a problem hiding this comment.
Reverted changes because it's out of scope for this PR
SNWCreations
left a comment
There was a problem hiding this comment.
Looks good. imo it is not necessary to introduce indent changes here.
And I think it is good for moving the codebase towards the lib even when on Spigot, since I know how the lib you used do its great work in my several projects. Also avoid unnecessary scoreboard data created by Citizens got saved by NMS when shutting down servers, provides a more clean experience.
| public void sendToPlayer(Player player, SendMode mode) { | ||
| // TODO: Currently not implemented | ||
| /*if (mode == SendMode.ADD_OR_MODIFY) { | ||
| delegateDisplay.refresh(); | ||
| delegateTeam.display(player, delegateDisplay); | ||
| } else { | ||
| delegateTeam.display(player, delegateTeam.defaultDisplay()); | ||
| }*/ | ||
| } |
There was a problem hiding this comment.
Is there a specific reason why this hasn't been implemented, or is it just an oversight?
There was a problem hiding this comment.
Agreed, TODOs should be finished
There was a problem hiding this comment.
Okay, after taking a look it just seems to be unnecessary. I had actually forgotten why I commented this out to begin with, but it was because the scoreboard library itself doesn't support not sending anything at all. There always has to be a TeamDisplay assigned to a ScoreboardTeam and all this method would do is just flip-flop between TeamDisplay objects. I commented it out and left the // TODO because I was going to look into it.
If you guys would prefer me to remove the TODO entirely, I can go ahead and do that.
|
Looks good, have you tested these changes actually load? |
|
Can merge if someone can confirm that these are tested changes on a Folia server |
In the meantime, did these changes tested on regular Spigot/Paper server as well? |
|
Will update this PR sometime tomorrow or the day after |
|
I have only tested these changes on Folia and Canvas, not on Paper or Spigot. I imagine everything will work the same because I pretty much copied-pasted the original implementations into delegate classes, but it's possible I may have made a typo or something. I can test on Paper sometime tomorrow or this weekend but my schedule has been a bit random lately. (I have been running this in production on my Canvas server.) |
|
If there's anything else that needs to be addressed, feel free to let me know and I'll update this PR as soon as possible. |
This PR adds support for the "scoreboardtrait" trait by introducing this library as a shaded dependency and sending all scoreboard and player-team related things with packets.
I made my changes in the
ScoreboardTraitclass under the assumption that Citizens2 only runs on 1.21.1+ based servers (based on the NMS implementations available).