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

Refactor power grid traverse code #1599

Conversation

AngelicosPhosphoros
Copy link
Contributor

@AngelicosPhosphoros AngelicosPhosphoros commented Jun 5, 2022

Summary

SUMMARY: Infrastructure "Refactor power grid traverse code"

Purpose of change

I was randomly reading code and found this function confusing. Refactored for readability.

Describe the solution

  • Split graph traversion and whatever caller wants to do with items.
  • Added comments in places where I failed to understand fast.
  • Removed amount updating from traversing and few hacks on call-sites caused by this.
  • Removed redundant "only vehicles" traversion method.
  • Removed raw pointers from public interface.

Describe alternatives you've considered

Testing

  • Run auto tests.
  • Loaded my save with power grid and vehicle with batteries, checked that power grid correctly shows power and charges/discharges.

Additional context

@AngelicosPhosphoros AngelicosPhosphoros force-pushed the improve_veh_grid_traverse branch from d6f0938 to f108f5e Compare June 5, 2022 15:02
@AngelicosPhosphoros
Copy link
Contributor Author

Experience of configuring Visual Studio and building the game was great for C++ project, take only half a hour. My thanks to whoever who made it so easy.

@AngelicosPhosphoros
Copy link
Contributor Author

@Coolthulhu May you approve CI run?

@AngelicosPhosphoros AngelicosPhosphoros force-pushed the improve_veh_grid_traverse branch from f108f5e to 40d7c60 Compare June 8, 2022 11:54
@Coolthulhu
Copy link
Member

Started the CI.

Will mergetest with next batch, assuming nothing explodes.

@Coolthulhu Coolthulhu self-assigned this Jun 8, 2022
@Coolthulhu
Copy link
Member

The builds complain about C++17 compatibility with noexcept. Might be easiest to leave noexcept out for now.

Also astyle.

I was randomly reading code and found this function confusing. Refactored for readability.

* Split graph traversion and whatever caller wants to do with items.
* Added comments in places where I failed to understand fast.
* Removed `amount` updating from traversing and few hacks on call-sites caused by this.
* Removed redundant "only vehicles" traversion method.
* Removed raw pointers from public interface.
@AngelicosPhosphoros AngelicosPhosphoros force-pushed the improve_veh_grid_traverse branch from 40d7c60 to 8b36799 Compare June 8, 2022 19:43
@AngelicosPhosphoros
Copy link
Contributor Author

@Coolthulhu runned astyle and removed noexcept.

@Coolthulhu Coolthulhu merged commit 8d144d2 into cataclysmbnteam:upload Jun 10, 2022
@AngelicosPhosphoros AngelicosPhosphoros deleted the improve_veh_grid_traverse branch June 10, 2022 22:37
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.

2 participants