Skip to content
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

Improve texts in the research view #2008

Merged
merged 4 commits into from
Jul 23, 2023

Conversation

lmoureaux
Copy link
Contributor

@lmoureaux lmoureaux commented Jul 22, 2023

Improve the following texts in the research view header following a comment by Blauwal on Discord:

  • The bulb production details (top left): add a missing newline when tech upkeep is enabled
  • The current status (top right in the progress bar): make it work better when tech leak and tech upkeep are used together
  • The number of bulbs/turns required to reach the current goal (bottom right): make it less confusing by taking the real cost of the current tech into account

Some small code improvements here and there as well.

I think the risks of backporting this outweigh the benefits.

With tech upkeep enabled, the text was looking like:

  Progress: %1 turn/advance (%1 bulb/turn)Bulbs produced per turn: %1

Add a newline after the closing parenthesis.
The price of a tech can change due to tech leak. In this case it isn't
researched immediately and the player may see 123/100 bulbs. Account for
this when calculating how many turns are left to discover the tech.
The research view has a text with the number of bulbs and turns needed
to achieve the current tech goal. This text had issues in multiple
situations:

* When no goal was set, it was showing "(0 steps - 0 bulbs - never)";
* It was never taking tech leak into account.

Solve these issues by considering the current research as the implicit
goal if none is set, and taking the actual cost of the current research
into account. The label still does not take tech leak into account for
the following techs, even if the player knows that the cost will be
lower (e.g. through diplomacy).
@lmoureaux lmoureaux requested a review from jwrober July 22, 2023 19:25
@jwrober
Copy link
Collaborator

jwrober commented Jul 23, 2023

Is there a savegame or something I can use to test this?

@lmoureaux
Copy link
Contributor Author

Unfortunately not, I was testing by observing Blauwal in LT79.

Copy link
Collaborator

@jwrober jwrober left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks like it works. I personally would like to actually see it work for real before we backport.

@jwrober jwrober added the back-port back-port candidate label Jul 23, 2023
@jwrober jwrober merged commit f86a922 into longturn:master Jul 23, 2023
17 of 18 checks passed
@lmoureaux lmoureaux deleted the bugfix/tech-turns-with-tech-leak branch July 25, 2023 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
back-port back-port candidate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants