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

Reverting waypoint names to default may change the name #3835

Closed
DorpsGek opened this issue May 13, 2010 · 5 comments
Closed

Reverting waypoint names to default may change the name #3835

DorpsGek opened this issue May 13, 2010 · 5 comments
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay

Comments

@DorpsGek
Copy link
Member

Krille opened the ticket and wrote:

r19817

Try this:
Start a new game
Build some rail
Build a waypoint
Change its name
Build another waypoint, making sure that it belongs to the same town
Revert the name of the first waypoint to default

You'll notice that the first waypoint now has a different name than before because its original name is already taken by the second waypoint.

I don't think this is intended? (One could think of it as a way to keep the numbers low)

I also took the chance to correct two unrelated comments.

Attachments

Reported version: trunk
Operating system: All


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

Krille wrote:

It turns out that the name still changed when one destroyed a waypoint with a lower number before reverting another waypoint to the default name. I've extended the patch to fix that.

The patch also keeps the waypoint centered in its window when the size is changed, as it happens for vehicle windows. Previously it didn't change on resize, but it jumped to the new position on rename.

I consider the patch as experimental since I'm not familiar with the saveload and windowing systems.

Another thing I noticed is that other windows (destination in vehicle windows, orders in order windows, ship list window, possibly more) don't update correctly after renaming a waypoint since they aren't marked dirty. I'm unsure what would be the best way to do this.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/3835#comment8035

@DorpsGek
Copy link
Member Author

SmatZ wrote:

It is not caused by recent changes. However, what would the solution be?
a) When "Waypoint # 2" is renamed, no waypoint will ever have name "Waypoint # 2"
or
b) When "Waypoint # 2" is renamed to "XXX, other waypoint is named "Waypoint # 2", and then "XXX" reset to default, it will have assigned new automatic name

Is there any reason one solution is better than the other?


This comment was imported from FlySpray: https://bugs.openttd.org/task/3835#comment8036

@DorpsGek
Copy link
Member Author

Krille wrote:

It's a fairly old problem as far as i could see in the svn history.

It currently does it like b). MakeDefaultName() doesn't look at the town_cn of of waypoints with a custom name. I couldn't find out why, maybe it was needed somehow by older versions of the code. Now, if you create a waypoint, rename it and create a second one, both will have town_cn 0.
When the name is reset it assigns a new number.

The patch will make it like a). It first makes sure that all waypoints have an unique town_cn by also considering those with a custom name. On reset we just have to free the custom name and the original name with the unique town_cn is displayed again.

The afterload code makes sure to renumber all waypoints from older games that have a custom name, since town_cn values may have been used multiple times. Those without a custom name are already fine.

The remaining parts of the patch fix things I found in the process. Two incorrect comments and autoscrolling on resize of waypoint windows.

I think a) is better since it's consistent with other objects (e. g. stations, vehicles) which also reset to their original name.


This comment was imported from FlySpray: https://bugs.openttd.org/task/3835#comment8037

@DorpsGek
Copy link
Member Author

Krille wrote:

I've extended the patch to set the town_cn of all affected waypoints to UINT16_MAX before renumbering them, as done with depots.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/3835#comment8043

@DorpsGek
Copy link
Member Author

Rubidium closed the ticket.

Reason for closing: Implemented

In r2081[34]. Thanks for the patch


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

@DorpsGek DorpsGek added Core flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay labels Apr 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay
Projects
None yet
Development

No branches or pull requests

1 participant