Skip to content
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

Per player power patch #71

Open
wants to merge 28 commits into
base: master
Choose a base branch
from

Conversation

CamperSamu
Copy link
Contributor

@CamperSamu CamperSamu commented Jul 3, 2022

Before working more on this I'd like to discuss various implementations of this.

For now I just added a way to modify via LuckPerms how much power can each user have via factions.power.modifier.<number>, but we can go further by moving the power attribute from the Faction to the User and add methods to calculate and manipulate power in a faction (it would be similar to this branch' Faction#calculateMaxPower).

I'd like to hear your point of view on this change.

(also this branch is based on CamperSamu:placeholders-patch since some work is needed on the placeholders too)
(now based on master)

@BlueZeeKing
Copy link
Collaborator

What is the use case for this? I don't really see a need for something like this

@CamperSamu
Copy link
Contributor Author

Servers might want to add extra power with roles (like VIPs or gamemode ranks), so adding a way to increase a player's power would be nice.

Instead, moving power to be per-player should prevent a faction's power to be completely drained by a single player committing suicide over and over.

@BlueZeeKing
Copy link
Collaborator

Instead, moving power to be per-player should prevent a faction's power to be completely drained by a single player committing suicide over and over.

This is something I think should be fixed, so per-player power could be a nice feature.

@CamperSamu
Copy link
Contributor Author

Great!
I'll start working on it again in the following days since during this week I won't be often at home.

@CamperSamu
Copy link
Contributor Author

This version of the patch is already testable.
CI Builds are available on my branch under github actions.

https://github.com/CamperSamu/factions/actions/runs/2624887857

# Conflicts:
#	src/main/java/io/icker/factions/api/persistents/Faction.java
#	src/main/java/io/icker/factions/command/InfoCommand.java
#	src/main/java/io/icker/factions/command/MemberCommand.java
#	src/main/java/io/icker/factions/core/FactionsManager.java
#	src/main/java/io/icker/factions/util/Migrator.java
Signed-off-by: Camper_Samu <[email protected]>
Signed-off-by: Camper_Samu <[email protected]>
@CamperSamu
Copy link
Contributor Author

CamperSamu commented Jul 10, 2022

I am removing base power since with the per-player power it's not possible to really implemet, but if you want we can add the base power to the faction's owner or make it common power.

Also I (obviously) had to change the f admin command to make it edit players and not factions.

…wer to prevent code duplication and missed formula changes (also this will allow for custom formulas in the future), NPE proofing in various commands and a bit of code annotation.

Signed-off-by: Camper_Samu <[email protected]>
@CamperSamu
Copy link
Contributor Author

In the meantime I'm doing a bit of code cleanup (also doing it on the code-cleanup branch, not frequently updated) and refactoring of minor stuff

@CamperSamu
Copy link
Contributor Author

Starting to work on commands and placeholders, then I'll playtest everything a bit; if you want to try it out and point out issues and missing features you're welcome!

Signed-off-by: Camper_Samu <[email protected]>
@CamperSamu
Copy link
Contributor Author

Last commit fixes broken factions by using f create on console and via /execute

@CamperSamu
Copy link
Contributor Author

now f info gives the player's power when used while not in a faction
image

@CamperSamu
Copy link
Contributor Author

(obviously the normal command stays unaffected)
image

Signed-off-by: Camper_Samu <[email protected]>
…h penalty for every death (pveDeathPenalty, false by default)

Signed-off-by: Camper_Samu <[email protected]>
@CamperSamu
Copy link
Contributor Author

Forgot to note, this includes the info command changes and the extra placeholders

@CamperSamu
Copy link
Contributor Author

CamperSamu commented Jul 10, 2022

The new placeholders are:

  • player_power_formatted
    • Shows the player power with a color that transitions between green and red
  • power_stats
    • Shows the current power stats
      • If the player is not in a faction it will show player power/player max power
      • If the player is in a faction, it will show faction power/faction required power/faction max power
  • power_stats_formatted
    • The same as power_stats, but with formatting like player power and the other pre-formatted placeholders

(Not in a Faction)
Not in a Faction

(In a Faction)
In a Faction

@CamperSamu
Copy link
Contributor Author

Rebalanced the default power values:

  • Each player has 20 power
  • Death penalty is now 5
  • Claim weight lowered to 2
  • Survival reward raised to 3

This should make everything work better after the power changes.

@CamperSamu
Copy link
Contributor Author

I'd say that this is now ready for PR!

Play around with it and tell me if anything else needs to be changed.
(Builds available in GH Actions)

@CamperSamu CamperSamu changed the title WIP: Per player power patch Per player power patch Jul 10, 2022
@CamperSamu CamperSamu marked this pull request as ready for review July 10, 2022 18:56
@CamperSamu
Copy link
Contributor Author

Forgot to mention, I've enabled maintainer commits to the branch, so if there's a small adjustment that you are willing to do yourself you can just checkout to my branch (or do it inside the web editor) and make the adjustment before merging the branch.

Copy link
Collaborator

@BlueZeeKing BlueZeeKing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks good, just a few things that need to be adjusted. Once thats done, I'm good to merge this.

@CamperSamu
Copy link
Contributor Author

Will work on it in the following days!
Also I've noticed weird behavior when the survival power bonus is applied on my server while testing, will double check that code ASAP before merging

@BlueZeeKing
Copy link
Collaborator

What problem was there with base power that caused you to remove it?

@CamperSamu
Copy link
Contributor Author

I am removing base power since with the per-player power it's not really possible to implemet elegantly, but if you want we can add the base power to the faction's owner or make it common power.

Base power doens't make sense in a system where the power belongs to single players; the only sensible place to put it would be in a common/faction power pool (that is against what this PR is trying to accomplish).
Handling a common pool of power is not as straight forward as removing power from a single user; a possible solution would be to give extra power to the faction owner, another one might be to deplete that pool when a user with 0 power dies or to remove a portion of common power for each death.

In my opinion this is confusing and it's better to remove base power.

@CamperSamu
Copy link
Contributor Author

(also sorry if I'm not quick at responding/working lately but I am currently in vacation with my family, both internet and time are kind of unreliable)

(check power range before subtracting)
Because of me changing how power is added the old code behaved in the wrong way.
Now I moved the range check outside of the method and relocated it into the reward/penalty code.

New Logic:
- Penalty
  - Check if the power is less or zero
    - If the power is less or zero quit out the fuction
  - Before adding power, check if the penalty makes the power value negative
    - If so, instead of subtracting the power power penalty, subtract the remaining power
    - Otherwise subtract the power penalty like normal
- Reward
  - Check if the power exceeds the max power
    - If the power exceeds the max power quit out the function
  - Before adding the reward, check if the current power + the reward overflows the max power
    - If so, add the remaining power (max power - power)
    - Otherwise, add power like normal.
@CamperSamu
Copy link
Contributor Author

Fixed the issue I had with power, I forgot to port the power cap checks to the penalty/reward

Copy link
Contributor Author

@CamperSamu CamperSamu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything should be good

@CamperSamu
Copy link
Contributor Author

Currently very, very busy, idk when I'll be able to bring everything up to speed, but it will be done eventually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Question] [Possible Feature Request] Does dying to a non-player entity reduce your power?
2 participants