-
Notifications
You must be signed in to change notification settings - Fork 708
Fix nation relation exploit 🔧 #2523
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: main
Are you sure you want to change the base?
Changes from all commits
9c1f7b4
543d0bb
f3d4ab2
ed71be8
1ce1eae
90dff31
6a83b35
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,18 @@ | ||
| import { Execution, Game, Gold, Player, PlayerID } from "../game/Game"; | ||
| import { | ||
| Difficulty, | ||
| Execution, | ||
| Game, | ||
| Gold, | ||
| Player, | ||
| PlayerID, | ||
| } from "../game/Game"; | ||
| import { PseudoRandom } from "../PseudoRandom"; | ||
| import { toInt } from "../Util"; | ||
|
|
||
| export class DonateGoldExecution implements Execution { | ||
| private recipient: Player; | ||
| private random: PseudoRandom; | ||
| private mg: Game; | ||
|
|
||
| private active = true; | ||
| private gold: Gold; | ||
|
|
@@ -16,8 +26,13 @@ export class DonateGoldExecution implements Execution { | |
| } | ||
|
|
||
| init(mg: Game, ticks: number): void { | ||
| this.mg = mg; | ||
| this.random = new PseudoRandom(mg.ticks()); | ||
|
|
||
| if (!mg.hasPlayer(this.recipientID)) { | ||
| console.warn(`DonateExecution recipient ${this.recipientID} not found`); | ||
| console.warn( | ||
| `DonateGoldExecution recipient ${this.recipientID} not found`, | ||
| ); | ||
| this.active = false; | ||
| return; | ||
| } | ||
|
|
@@ -32,7 +47,11 @@ export class DonateGoldExecution implements Execution { | |
| this.sender.canDonateGold(this.recipient) && | ||
| this.sender.donateGold(this.recipient, this.gold) | ||
| ) { | ||
| this.recipient.updateRelation(this.sender, 50); | ||
| // Give relation points based on how much gold was donated | ||
| const relationUpdate = this.calculateRelationUpdate(this.gold); | ||
| if (relationUpdate > 0) { | ||
| this.recipient.updateRelation(this.sender, relationUpdate); | ||
| } | ||
| } else { | ||
| console.warn( | ||
| `cannot send gold from ${this.sender.name()} to ${this.recipient.name()}`, | ||
|
|
@@ -41,6 +60,33 @@ export class DonateGoldExecution implements Execution { | |
| this.active = false; | ||
| } | ||
|
|
||
| getGoldChunkSize(): Gold { | ||
| const { difficulty } = this.mg.config().gameConfig(); | ||
| switch (difficulty) { | ||
| case Difficulty.Easy: | ||
| return BigInt(this.random.nextInt(1, 2_500)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we scale these values with tick number, or with number of factories/ports on the map? The more economy buildings there are, the less valuable donations become, and in the endgame,
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thats an interesting idea, but then players would have to do calculations if they want to "buy" a good relation with a nation. I'm not sure if thats good or bad
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My thought was that the more it hurts you/the more it helps the nation, the more it raises the relation, so players wouldn't necessarily have to do calculations, they'd just give as much as they are comfortable with. But this was just an idea to be clear, nothing necessary. |
||
| case Difficulty.Medium: | ||
| return BigInt(this.random.nextInt(2_500, 5_000)); | ||
| case Difficulty.Hard: | ||
| return BigInt(this.random.nextInt(5_000, 12_500)); | ||
| case Difficulty.Impossible: | ||
| return BigInt(this.random.nextInt(12_500, 25_000)); | ||
| default: | ||
| return 2_500n; | ||
| } | ||
| } | ||
|
|
||
| calculateRelationUpdate(goldSent: Gold): number { | ||
| const chunkSize = this.getGoldChunkSize(); | ||
| // Calculate how many complete chunks were donated | ||
| const chunks = Number(goldSent / chunkSize); | ||
| // Each chunk gives 5 relation points | ||
| const relationUpdate = chunks * 5; | ||
| // Cap at 100 relation points | ||
| if (relationUpdate > 100) return 100; | ||
| return relationUpdate; | ||
| } | ||
|
|
||
| isActive(): boolean { | ||
| return this.active; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,10 @@ | ||
| import { Execution, Game, Player, PlayerID } from "../game/Game"; | ||
| import { Difficulty, Execution, Game, Player, PlayerID } from "../game/Game"; | ||
| import { PseudoRandom } from "../PseudoRandom"; | ||
|
|
||
| export class DonateTroopsExecution implements Execution { | ||
| private recipient: Player; | ||
| private random: PseudoRandom; | ||
| private mg: Game; | ||
|
|
||
| private active = true; | ||
|
|
||
|
|
@@ -12,8 +15,13 @@ export class DonateTroopsExecution implements Execution { | |
| ) {} | ||
|
|
||
| init(mg: Game, ticks: number): void { | ||
| this.mg = mg; | ||
| this.random = new PseudoRandom(mg.ticks()); | ||
|
|
||
| if (!mg.hasPlayer(this.recipientID)) { | ||
| console.warn(`DonateExecution recipient ${this.recipientID} not found`); | ||
| console.warn( | ||
| `DonateTroopExecution recipient ${this.recipientID} not found`, | ||
| ); | ||
| this.active = false; | ||
| return; | ||
| } | ||
|
|
@@ -27,11 +35,17 @@ export class DonateTroopsExecution implements Execution { | |
|
|
||
| tick(ticks: number): void { | ||
| if (this.troops === null) throw new Error("not initialized"); | ||
|
|
||
| const minTroops = this.getMinTroopsForRelationUpdate(); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this variable can be inlined
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had it inlined, but then coderabbit rightfully said that we need to calculate the troops BEFORE donating them. |
||
|
|
||
| if ( | ||
| this.sender.canDonateTroops(this.recipient) && | ||
| this.sender.donateTroops(this.recipient, this.troops) | ||
| ) { | ||
| this.recipient.updateRelation(this.sender, 50); | ||
| // Prevent players from just buying a good relation by sending 1% troops. Instead, a minimum is needed, and it's random. | ||
| if (this.troops >= minTroops) { | ||
| this.recipient.updateRelation(this.sender, 50); | ||
| } | ||
| } else { | ||
| console.warn( | ||
| `cannot send troops from ${this.sender} to ${this.recipient}`, | ||
|
|
@@ -40,6 +54,40 @@ export class DonateTroopsExecution implements Execution { | |
| this.active = false; | ||
| } | ||
|
|
||
| getMinTroopsForRelationUpdate(): number { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you use a switch case for this
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! |
||
| const { difficulty } = this.mg.config().gameConfig(); | ||
| const recipientMaxTroops = this.mg.config().maxTroops(this.recipient); | ||
|
|
||
| switch (difficulty) { | ||
| // ~7.7k - ~9.1k troops (for 100k troops) | ||
| case Difficulty.Easy: | ||
| return this.random.nextInt( | ||
| recipientMaxTroops / 13, | ||
| recipientMaxTroops / 11, | ||
| ); | ||
| // ~9.1k - ~11.1k troops (for 100k troops) | ||
| case Difficulty.Medium: | ||
| return this.random.nextInt( | ||
| recipientMaxTroops / 11, | ||
| recipientMaxTroops / 9, | ||
| ); | ||
| // ~11.1k - ~14.3k troops (for 100k troops) | ||
| case Difficulty.Hard: | ||
| return this.random.nextInt( | ||
| recipientMaxTroops / 9, | ||
| recipientMaxTroops / 7, | ||
| ); | ||
| // ~14.3k - ~20k troops (for 100k troops) | ||
| case Difficulty.Impossible: | ||
| return this.random.nextInt( | ||
| recipientMaxTroops / 7, | ||
| recipientMaxTroops / 5, | ||
| ); | ||
| default: | ||
| return recipientMaxTroops / 11; | ||
| } | ||
| } | ||
|
|
||
| isActive(): boolean { | ||
| return this.active; | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
Would it be better to put this into defaultConfig?
You could also use the unused difficultyModifier function if you want, but it's not needed.
Some goes for the troop switch
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.
Uh I don't know... Putting something in defaultConfig is probably most helpful if its used in multiple places?
If we want all constants in defaultConfig then a lot is missing there