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

Codechange: Make the road vehicle path cache behave more like a std::deque #12275

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Kuhnovic
Copy link
Contributor

@Kuhnovic Kuhnovic commented Mar 11, 2024

Motivation / Problem

The Road Vehicle Path Cache is actually two std::deque's in one. That's a bit silly, but it is done to make saveload work. But the silliness has propaged into other parts of the code, where you basically have to push and pop items to both queues,

Description

I tried to keep the two-queues-in-one an implementation detail. From the caller's perspective it works just like a single std::deque. Sort of, because I only implemented the functions that were needed ;)

Limitations

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR touches english.txt or translations? Check the guidelines
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, game_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

@PeterN
Copy link
Member

PeterN commented Mar 19, 2024

It would actually be better to replace the two deque system. That was an oversight on my part that needlessly uses quite a lot of memory for each RoadVehicle object.

Some other container might be perfectly fine here too, I don't think it even needs to be a deque. With some reworking of how the node path is traversed, a simple vector should work as it only needs to be read from one end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants