Skip to content

Attempt to fix issues with BuildLimitGroup#2285

Open
Coronia wants to merge 2 commits into
Phobos-developers:developfrom
Coronia:build-limit-group-refractor
Open

Attempt to fix issues with BuildLimitGroup#2285
Coronia wants to merge 2 commits into
Phobos-developers:developfrom
Coronia:build-limit-group-refractor

Conversation

@Coronia

@Coronia Coronia commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Previously BuildLimitGroup has several issues. E.g. Techno is still buildable when it's reaching the limit but stuck at 50% building progress under certain conditions. This is an attempt to fix some of the issues, which refractor the code of BuildLimitGroup on upgrade to make it work independently from other techno

@Coronia Coronia added ❓Phobos bug Something isn't working properly Needs testing ⚙️T1 T1 maintainer review is sufficient No Documentation Needed No documentation needed whatsoever Bugfix This is a bugfix that does not need documentation beyond mention in changelog labels Jul 7, 2026
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

Nightly build for this pull request:

This comment is automatic and is meant to allow guests to get latest nightly builds for this pull request without registering. It is updated on every successful build.

@Coronia
Coronia marked this pull request as ready for review July 8, 2026 01:53
@Coronia

Coronia commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

so far no issue has occured in testing, but I'm not sure whether they're all resolved. Would require some more test and review

@TaranDahl

Copy link
Copy Markdown
Contributor

It would be best to provide an analysis of the errors in the previous implementation and their resulting consequences. Simply put, it should cover "how things were done before", "why the previous approach was wrong", and "how things are done now".
I cannot review the code without context.

@Coronia

Coronia commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

tbh I don't quite understand the idea of original author either, so it's just about fixing these 2 flaws:

  • whatever the idea was, it's bad to recalculate and reassign EAX for 3 times in a row. It should be squeezed into 1 calculation if anything
  • this function is only supposed to work for Upgrades, while the current implementation make it work for other techno too

So what I'm doing here is basically merging all EAX calculation into one function, and make it only affect Upgrade

Anyway, testing suggested that it can fix the issue of production being interrupted when the factories got changed (e.g. building a new one, EMP a current one etc). However, the 50% production stuck issue still happens sometime. Not sure if I have a way to fix it for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bugfix This is a bugfix that does not need documentation beyond mention in changelog Needs testing No Documentation Needed No documentation needed whatsoever ❓Phobos bug Something isn't working properly ⚙️T1 T1 maintainer review is sufficient

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants