Skip to content

Update OnProductionComplete.tsx#608

Open
EndrosG wants to merge 2 commits intofishpondstudio:mainfrom
EndrosG:patch-7
Open

Update OnProductionComplete.tsx#608
EndrosG wants to merge 2 commits intofishpondstudio:mainfrom
EndrosG:patch-7

Conversation

@EndrosG
Copy link
Copy Markdown
Contributor

@EndrosG EndrosG commented Apr 1, 2026

Osaka & GoldenGate had wrong effect: PowerPlant
Correct: PowerGrid -- compare ItaipuDam and AkademikLomonosov

PowerPlant = adds another +1 range around the tile, making range 2 effectively range 3.

Osaka & GoldenGate had wrong effect: PowerPlant
Correct: PowerGrid -- compare ItaipuDam and AkademikLomonosov

PowerPlant = adds another +1 range around the tile, making range 2 effectively range 3.
@insraq
Copy link
Copy Markdown
Contributor

insraq commented Apr 1, 2026

This fix is wrong. The description says 2-tile range, and it adds power plants with 1-tile range (which is in effect 2-tile range).

This is what makes maintaining the code hard ... two wonders have "range -1 with powerPlant" while two others have "range with powerGrid".
When I changed "grid.getRange ..." in the whole file to the buildingRange ... Osaka and GoldenGate became wrong because of this inconsistent coding.

Changing it from "range -1 with powerPlant" to "range with powerGrid" will make future changes easier because the real range is in the code.
@EndrosG
Copy link
Copy Markdown
Contributor Author

EndrosG commented Apr 1, 2026

(fixed it, see detailed comment on 763b7c1)

@insraq
Copy link
Copy Markdown
Contributor

insraq commented Apr 1, 2026

Your change is not synced in this PR.

@insraq
Copy link
Copy Markdown
Contributor

insraq commented Apr 1, 2026

Also I would advise against making the change because there're subtle differences and this change might break someone's setup.

@EndrosG
Copy link
Copy Markdown
Contributor Author

EndrosG commented Apr 1, 2026

Your change is not synced in this PR.

I do not understand this. The PR shows 4 changed lines. That's what is intended.

Also I would advise against making the change because there're subtle differences and this change might break someone's setup.

It makes all 4 Wonders with power supply effect work the same way.
Right now: GoldenGateBridge + OsakaCastle = old code, ItaipuDam + AkademikLomonosov = new code.
Same result: all tiles in +2 range become "powerGrid", just with different computation effort because old takes an extra function evaluating "powerPlants" tiles and populating "powerGrid" around "powerPlants".
Old code: make 6 tiles to powerPlants, and for each of the 6 make their adjacent 6 tiles to powerGrid with 7 loop over the map and still ... in the end ... just +18 tiles "powerGrid"
New code: make 18 tiles to powerGrid in one loop.

What kind of setup can be broken the way how ItaipuDam and AkademikLomonosov work compared to GoldenGateBridge + OsakaCastle?

@insraq
Copy link
Copy Markdown
Contributor

insraq commented Apr 1, 2026

Sorry I was reading an outdated webpage.

I don't remember exactly why it's done this way but I remember there's a reason because I had the same question as you a while ago but decided to keep it. Maybe the scenario no longer applies since electricity has been completely refactored.

If we were to adopt the change, it would need to be thoroughly tested in beta.

@EndrosG
Copy link
Copy Markdown
Contributor Author

EndrosG commented Apr 1, 2026

I'd suggest then that you ask your moderator community like Devonin & co if they know of any difference regarding power supply from the four wonders, specifically from the older two versus newer two as pointed out above.

From my understanding there is no difference. I applied the change in my MOD. But again, my understanding can be uncomplete.
If nobody else is aware of any difference between the two ways, then we should IMHO adopt the change in order to harmonize it to one standard - and to allow the next following change to using const buildingRange throughout the entire file as proposed via Discord.

Until then this PR should just stay open to remind us on that question.

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