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

Some code changes #5260

Closed
DorpsGek opened this issue Aug 1, 2012 · 3 comments
Closed

Some code changes #5260

DorpsGek opened this issue Aug 1, 2012 · 3 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

DorpsGek commented Aug 1, 2012

juanjo opened the ticket and wrote:

Some patches removing some unused pieces of code.

About ships:
- You can use IsValidTile to check if a new tile is valid.
- GetNewDirectionFromTiles is equivalent to find a DiagdirBetweenTile. If the problem is to check the assert included in GetNewDirectionFromTiles, it is calculated again some lines after getting the direction.

About groups:
- If group data doesn't change, there is no need to invalidate data. If it changes because you have called an add_vehicle_to_group_command, the add_vehicle_..._command will invalidate the data. Maybe this shouldn't be removed for keeping code readable, but it is useless anyway.
- No need to call ComputeGroupSize there.
- GetGroupArraySize isn't used anymore. And if needed, pools provide a better way to do so.

Attachments

Reported version: trunk
Operating system: All

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

Yexo wrote:

I've applied the IsValidTile and RemoveGetGroupArraySize patches.

I think the RemoveCallComputeGroupInfoSize patch is wrong. The next line uses this->tiny_step_height which is initialized by ComputeGroupInfoSize().

This comment was imported from FlySpray: https://bugs.openttd.org/task/5260#comment11531

@DorpsGek
Copy link
Member Author

juanjo wrote:

Yep. That one was incorrect. Probably ComputeGroupInfoSize() is called earlier, but it is no excuse to remove it.

There's also the RemoveGetNewDirection patch. I guess it needs more deep thinking if that can be simplified, but I've been testing it and it works.

This comment was imported from FlySpray: https://bugs.openttd.org/task/5260#comment11535

@DorpsGek
Copy link
Member Author

andythenorth closed the ticket.

Reason for closing: Out of date

No progress on this in 5 years. Let it slip away peacefully.

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

@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
Assignees
No one assigned
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
Milestone
No milestone
Development

No branches or pull requests
1 participant
@DorpsGek