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

Taxes #62

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Taxes #62

wants to merge 6 commits into from

Conversation

siBalda
Copy link

@siBalda siBalda commented Mar 2, 2024

Added taxation

Added taxation
Applied taxes on profits
Now /checkprofit can be used on other people if you have the permission "ecs.checkprofits.other". The player target has to be included at the end of the command. Example: "/cp p 1 Player".
Copy link
Owner

@ItzAmirreza ItzAmirreza left a comment

Choose a reason for hiding this comment

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

Needs changes

}
}

deposit(price, deposit);
Copy link
Owner

Choose a reason for hiding this comment

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

This line generates a bug that gives player 2 times deposit instead of one. Could even call it a malicious code...

The deposit(taxes ,Bukkit.getOfflinePlayer(Config.leader)); section doesn't return when deposit is done, and then goes to the next deposit function and do it again.

Copy link
Owner

Choose a reason for hiding this comment

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

Also comments on sections are appreciated, you must explain in simple terms what you are doing line by line

Copy link
Author

Choose a reason for hiding this comment

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

Your correction has no sense because the first deposit is for the "State": it deposits the taxes to the leader who represents the state. The second deposit is for the seller. I would like to ask you to test it first and then write that there is a "malicious code" or a "bug". Indeed me and Elito have already tested it 3 months ago (.......) and it works.

Copy link
Owner

Choose a reason for hiding this comment

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

I don't understand this section much since Elito coded it, needs @ElitoGame 's review.

//if the command is executed on another player
Player target = null;
int isOther = 0;
if(p.hasPermission("ecs.checkprofits.other")){
Copy link
Owner

Choose a reason for hiding this comment

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

What if player doesn't have the permission and target stays null?

taxes:
sell-to-shop-tax-percent: 0
buy-from-shop-tax-percent: 0
collecting-player-name: ""
Copy link
Owner

Choose a reason for hiding this comment

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

tax officer name would better? Not sure if we have to even include this since it's just taxes and everyone knows the server is collecting the tax.

@ItzAmirreza ItzAmirreza mentioned this pull request Apr 23, 2024
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.

2 participants