Navigation Menu

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

Jump to bottom

Returning negative values from the refit_cost callback #5297

Closed
DorpsGek opened this issue Sep 14, 2012 · 4 comments
Closed

Returning negative values from the refit_cost callback #5297

DorpsGek opened this issue Sep 14, 2012 · 4 comments
Labels
component: interface This is an interface issue flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)

Comments

@DorpsGek
Copy link
Member

Supercheese opened the ticket and wrote:

When returning negative values from the refit_cost callback in OTTD (tested with r24517), the functionality is as intended - namely, the player is refunded money. However, the refit window displays: "Cost of refit: {RED}-$XX,XXX", as shown in the attached screenshot. That seems rather silly IMO, and should probably be changed to {GREEN} text along with some indication of a refund.

For a .grf that exhibits this behavior, see the Fake Subways .grf: http://www.tt-forums.net/viewtopic.php?f=26&t=62672
(That .grf is attached to this report.)

The source and compilation instructions are attached to the second post of that thread, but I've also attached the resultant .nml file to this report, for quick viewing of the code. You'll of course need the remaining sources from the thread to compile the functional .grf. (Search for "refit_cost_switch" to quickly arrive at the relevant switch block.)

Anyway, steps to reproduce:

  1. Load Fake Subways .grf in a new game
  2. Build road depot
  3. Purchase a subway
  4. Refit to more carriages
  5. Refit again down to fewer carriages

That's about it.

Attachments

Reported version: trunk
Operating system: Windows

This issue was imported from FlySpray: https://bugs.openttd.org/task/5297
@DorpsGek
Copy link
Member Author

Supercheese wrote:

Forgot .grf, here it is.

Attachments

This comment was imported from FlySpray: https://bugs.openttd.org/task/5297#comment11506

@DorpsGek
Copy link
Member Author

Terkhen wrote:

Thanks for the report.

I'm sorry for the lateness in my answer. Last weekend I had a working patch to solve this issue, but I did not commit it because I was concerned with a not so trivial detail: aesthetics. I am not sure about which colour scheme would be better for this. The options I'm currently considering are:

  1. Gold for refit cost, gold for refit income
  2. Red for refit cost, gold for refit income
  3. Gold for refit cost, green for refit income
  4. Red for refit cost, green for refit income

Option 2) feels more readable for me, therefore I'm somewhat inclined to use it.

I'm attaching two screenshots that show option 1) and option 4) for comparison.

Attachments

This comment was imported from FlySpray: https://bugs.openttd.org/task/5297#comment11523

@DorpsGek
Copy link
Member Author

Supercheese wrote:

Well, immediately after the user selects a refit and confirms that selection, either a Red "Cost" or a Green "Income" will appear above the depot. If one wishes to be maximally consistent with this, the refit costs should be displayed in red and green as well (and gold for zero cost, I suppose). I personally lean towards this, but I am quite open to other arguments.

As a side note, you seem to have found a bug in the .grf related to rounding errors with negative numbers. Interesting how a silly thing such as a negative sign inside or outside the parentheses can result in rounding errors!

This comment was imported from FlySpray: https://bugs.openttd.org/task/5297#comment11534

@DorpsGek
Copy link
Member Author

Terkhen closed the ticket.

Reason for closing: Fixed

In r24544, with option 4).

This comment was imported from FlySpray: https://bugs.openttd.org/task/5297

@DorpsGek DorpsGek added component: interface This is an interface issue flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) bug labels Apr 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
component: interface This is an interface issue flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@DorpsGek