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

730 cleanup loading custom path manager #1072

Merged
merged 24 commits into from
Apr 12, 2021

Conversation

kianzarrin
Copy link
Collaborator

@kianzarrin kianzarrin commented Apr 8, 2021

  • Fixed master Debug build broken.

fixes #730

  • encapsulated code for loading/unloading CustomPathManager.
  • TMPE can be removed mid-game (without the need to replace it with anther TMPE).
  • bugfix: unloading TMPE replaces back the stock path find.
  • removed redundant(no profile uses it) / stale PF_DIJKSTRA (it does not even build)

Test passed:

  1. Load Game with TMPE
  2. apply TMPE lane connections to force cars to change
  3. observe that cars change lanes
  4. delete TMPE
  5. observe that cars no longer change lanes.
  6. build TMPE again (hot swap)
  7. observe that cars change lanes again.

https://ci.appveyor.com/api/projects/krzychu124/tmpe/artifacts/TMPE.zip?branch=730-cleanup-loading-CustomPathManager

blocks #1076 #1077

@kianzarrin kianzarrin added enhancement Improve existing feature code cleanup Refactor code, remove old code, improve maintainability labels Apr 8, 2021
@kianzarrin kianzarrin self-assigned this Apr 8, 2021
@kianzarrin kianzarrin requested a review from DaEgi01 April 8, 2021 12:58
@kianzarrin kianzarrin marked this pull request as draft April 8, 2021 13:12
@kianzarrin kianzarrin marked this pull request as ready for review April 9, 2021 07:15
Copy link
Collaborator

@kvakvs kvakvs left a comment

Choose a reason for hiding this comment

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

Formatting is off the rails.
I believe some debug log messages have no value (the operations cannot fail, and the time they appear in log is not helping anything, just noise).

TLM/TLM/Custom/PathFinding/CustomPathManager.cs Outdated Show resolved Hide resolved
TLM/TLM/Custom/PathFinding/CustomPathManager.cs Outdated Show resolved Hide resolved
TLM/TLM/Custom/PathFinding/CustomPathManager.cs Outdated Show resolved Hide resolved
TLM/TLM/Custom/PathFinding/CustomPathManager.cs Outdated Show resolved Hide resolved
TLM/TLM/Custom/PathFinding/CustomPathManager.cs Outdated Show resolved Hide resolved
TLM/TLM/Custom/PathFinding/CustomPathManager.cs Outdated Show resolved Hide resolved
TLM/TLM/Custom/PathFinding/CustomPathManager.cs Outdated Show resolved Hide resolved
@kianzarrin
Copy link
Collaborator Author

@kvakvs I fixed the spaces and remove most debug logs. but I think it is a good idea to keep the debug logs that help us track CustomPathManager VS Stock PathManager.

Copy link
Collaborator

@kvakvs kvakvs left a comment

Choose a reason for hiding this comment

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

Looks good

TLM/TLM/Custom/PathFinding/CustomPathManager.cs Outdated Show resolved Hide resolved
TLM/TLM/Custom/PathFinding/CustomPathManager.cs Outdated Show resolved Hide resolved
TLM/TLM/Custom/PathFinding/CustomPathManager.cs Outdated Show resolved Hide resolved
@kianzarrin kianzarrin added the Blocking Another issue depends on this issue. label Apr 12, 2021
Copy link
Member

@krzychu124 krzychu124 left a comment

Choose a reason for hiding this comment

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

Cool 👍

Copy link
Contributor

@chameleon-tbn chameleon-tbn left a comment

Choose a reason for hiding this comment

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

lgtm

@kianzarrin kianzarrin merged commit 57c0e92 into master Apr 12, 2021
@kianzarrin kianzarrin deleted the 730-cleanup-loading-CustomPathManager branch April 12, 2021 10:05
@originalfoo originalfoo added this to the 11.6.0 milestone Jan 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocking Another issue depends on this issue. code cleanup Refactor code, remove old code, improve maintainability enhancement Improve existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clean up Loading extension CustomPathManager
6 participants