refactor: migrate states#1289
Conversation
✅ Deploy Preview for afmg ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
Migrates the legacy States generator from public/modules JavaScript into the TypeScript/Vite module pipeline, aligning runtime loading with the ongoing TS migration and updating shared type definitions.
Changes:
- Added a new
src/modules/states-generator.tsand wired it intosrc/modules/index.ts - Removed the legacy
public/modules/states-generator.jsand stopped loading it fromsrc/index.html - Extended global / packed-graph TypeScript types to include state-related data and globals used by the migrated module
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types/global.ts | Adds missing global declarations (COA, options) used by the migrated TS module |
| src/types/PackedGraph.ts | Extends packed graph typing to include states and additional cell fields used by states logic |
| src/modules/states-generator.ts | New TypeScript implementation of the States generator (ported from the removed JS module) |
| src/modules/index.ts | Ensures the new TS States module is loaded via side-effect import |
| src/index.html | Removes loading of the legacy modules/states-generator.js script |
| public/modules/states-generator.js | Deleted legacy JS implementation now replaced by the TS module |
| .filter((d) => d) | ||
| .forEach((v) => { | ||
| attackers.push(v); | ||
| dp += states[v].area! * states[v].expansionism!; |
There was a problem hiding this comment.
In the attacker-allies section, when an ally’s vassals join the attackers, their power is currently added to dp (defenders power). This should increase ap (attackers power); otherwise subsequent balance checks and diplomacy outcomes are skewed.
| dp += states[v].area! * states[v].expansionism!; | |
| ap += states[v].area! * states[v].expansionism!; |
There was a problem hiding this comment.
this review makes me cautious. I will check again tomorrow that i did not do any error here.
| : states[f] | ||
| .neighbors!.map((n) => states[n].neighbors) | ||
| .join("") | ||
| .includes(String(t)); |
There was a problem hiding this comment.
neibOfNeib detection uses .map(...).join("").includes(String(t)), which can produce false positives (e.g., neighbor list [11] makes includes("1") true). Consider using a proper nested membership check (e.g., some + includes) or a Set of second-degree neighbors.
| : states[f] | |
| .neighbors!.map((n) => states[n].neighbors) | |
| .join("") | |
| .includes(String(t)); | |
| : states[f].neighbors!.some((n) => states[n].neighbors!.includes(t)); |
Description
Type of change
Versioning