feat: player abilities APIs#843
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1bc1675f7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * | ||
| * @return {@code true} if building is currently allowed, {@code false} otherwise | ||
| */ | ||
| boolean canBuild(); |
There was a problem hiding this comment.
We don't need the following getter/setter of each individual ability, just let the user use addAbility/removeAbility method to make the Player class clean
There was a problem hiding this comment.
getters are not simple ability lookup. even with BUILD ability present, client would still not be able (and thus server should not it allow to) to place blocks if it's in adventure mode, spectator mode, or immutableWorld is sent. getters are aware of this, while hasPermission is separate lookup with different semantics.
setters are purely setters, but with getters setters are usually expected too, and setCanBuild(false) feels cleaner than setAbility(PlayerAbility.BUILD, false), which requires user to know about the abilities API or specific enums, which might even be considered an implementation detail.
49b7e96 to
d876bf9
Compare
7d606e5 to
70ad58b
Compare
…T in favor of Permissions.ABILITY_OPERATOR_COMMAND_QUICK_BAR
smartcmd
left a comment
There was a problem hiding this comment.
Should also rename the newly added methods in Player
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a86bc3bb69
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
done |
1498d1e to
6a6ce08
Compare
| removeViewer(viewer); | ||
| return addViewer(viewer); | ||
| } | ||
| if (viewer instanceof Player player && this instanceof BlockContainer && !player.canOpenContainers()) { |
There was a problem hiding this comment.
Shouldn't put the check here, Container.addViewer() should always success in my opinion
There was a problem hiding this comment.
why addViewer should succeed if player is not allowed to open and view containers?
There was a problem hiding this comment.
Handler of ContainerOpenEvent event would literally see event.getPlayer().canOpenContainers() as false, is it really expected behavior?
There was a problem hiding this comment.
Maybe add canOpenContainers in container open event?
There was a problem hiding this comment.
I'm not really understanding the point of that. why not invoke it when a player tries to open a chest that's blocked by a block above then? maybe plugins should use the block interaction event in such cases?
| removeViewer(viewer); | ||
| return addViewer(viewer); | ||
| } | ||
| if (viewer instanceof Player player && !player.canOpenContainers()) { |
|
Resolve conflicts |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 45 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
# Conflicts: # api/src/main/java/org/allaymc/api/player/PlayerData.java # server/src/main/java/org/allaymc/server/command/defaults/GameTestCommand.java # server/src/main/java/org/allaymc/server/network/processor/ingame/PlayerAuthInputPacketProcessor.java
4e7823d to
1c9ae04
Compare
done |
Massive ability refactor.
Replaces old hardcoded game mode and permission checks with a server-tracked
PlayerAbilityset on each player. Abilities are now part of the player API, broadcasted to all clients, validated server-side, and persisted in player data.The new system adds explicit ability state tracking and validating for all vanilla abilities like building and mining. Players now expose ability-focused APIs such as
setAbility,setAbilities,hasAbility, client-synced gamemode-aware checkers such ascanBuild,canMine,canOpenContainers, ability setters such assetCanFly,setCanMine. Maps cleanly to operator and gamemode updates.Also improves enforcement and fixes several gameplay/security issues: blocks cannot be placed in adventure mode now, all world interactions are prevented in spectator mode, player abilities and permission are globally broadcasted and can be modified by operators.
Additional changes:
setImmutableWorld, which maps toUpdateAdventureSettingsPacket#immutableWorld(active in adventure and spectator modes)setAlwaysFlying, which keeps the client in flight (active in spectator mode)Events:
PlayerAbilitiesUpdateEventfires after a player’s abilities changePlayerAbilitiesUpdateRequestEventfires when an operator requests an ability update for another player through the client permission UIBreaking changes:
Permissions.ABILITY_FLY_SURVIVAL,Permissions.ABILITY_FLY_CREATIVE,Permissions.ABILITY_FLY_ADVENTURE, since it's now tracked by abilitiesPlayer#viewPlayerPermission(...)renamed toPlayer#viewPlayerAbilities(...)