From 91f1748d70ad0148d4af50ccf28a64b02518971f Mon Sep 17 00:00:00 2001 From: VariableVince <24507472+VariableVince@users.noreply.github.com> Date: Sun, 26 Oct 2025 22:48:54 +0100 Subject: [PATCH] Perf: remove redundant code from PlayerExecution (#2299) ## Description: Remove some lines of code that do nothing in surroundedBySamePlayer, gain a small bit of performance back. The code says "if it is an ocean shore tile but not an ocean shore tile" which can never be true. (Why where the lines there and why can they be removed: Before you were able to boat on a lake, you had no escape from your land if it bordered a lake and no ocean. So it was logical, if you bordered a lake with your cluster, to still treat it as fully surrounded annexable land. This is why this code was added in [this commit](https://github.com/openfrontio/OpenFrontIO/commit/ee56d687484131c092599434247b3a036e0e9668) back then: `const isOceanShore = this.mg.isOceanShore(tile); if (this.mg.isShore(tile) && !isOceanShore) { continue; }` But [this newer commit](https://github.com/openfrontio/OpenFrontIO/commit/c1383d76f1f76c5e5f9be3a6f30b8f3010258540#diff-fb1101a2b50dd7c353d082ff7a3351cff5469b8249b3ebca91c10573a3dfaaf1) made it so you could from then on boat on lakes. So you have an escape from your cluster since then. And just like being on the edge of the map or when bordering the ocean, this means your cluster won't get annexed. However, while the updated code for PlayerExecution in the last commit does its work as intended (it does not exclude lake shore tiles anymore)... The code contradicts itself: `const isOceanShore = this.mg.isOceanShore(tile); if (this.mg.isOceanShore(tile) && !isOceanShore) { continue;}` Conclusion: the code on lines 136-138 can be deleted because it literally says "if it is an ocean shore tile but not an ocean shore tile" which can never be true. Also remove the const and check isOceanShore directly in the if statement.) ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced ## Please put your Discord username so you can be contacted if a bug or regression is found: tryout33 --- src/core/execution/PlayerExecution.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/core/execution/PlayerExecution.ts b/src/core/execution/PlayerExecution.ts index bc2491a72..a8cc3fb42 100644 --- a/src/core/execution/PlayerExecution.ts +++ b/src/core/execution/PlayerExecution.ts @@ -132,12 +132,8 @@ export class PlayerExecution implements Execution { private surroundedBySamePlayer(cluster: Set): false | Player { const enemies = new Set(); for (const tile of cluster) { - const isOceanShore = this.mg.isOceanShore(tile); - if (this.mg.isOceanShore(tile) && !isOceanShore) { - continue; - } if ( - isOceanShore || + this.mg.isOceanShore(tile) || this.mg.isOnEdgeOfMap(tile) || this.mg.neighbors(tile).some((n) => !this.mg?.hasOwner(n)) ) {