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 OptionsManager.cs #1321

Merged
merged 3 commits into from
Jan 29, 2022
Merged

Refactor OptionsManager.cs #1321

merged 3 commits into from
Jan 29, 2022

Conversation

originalfoo
Copy link
Member

@originalfoo originalfoo commented Jan 27, 2022

TMPE.zip

Refactors OptionsManager.cs to use specific index references, resulting in cleaner and more maintainable code.

This is a prerequisite for following PRs:

- Code provided by kvakvs
@originalfoo originalfoo added the code cleanup Refactor code, remove old code, improve maintainability label Jan 27, 2022
@originalfoo originalfoo added this to the 11.6.5 milestone Jan 27, 2022
@originalfoo originalfoo self-assigned this Jan 27, 2022
@originalfoo originalfoo mentioned this pull request Jan 27, 2022
1 task
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 great
Hope that i didn't mess up index values anywhere

@originalfoo
Copy link
Member Author

Heh, I've not checked through it all yet - just copied & pasted from your repo. It's compiling though and seems to work on cursory glance in-game.

- also manually checked all options are working
@originalfoo originalfoo added the Blocking Another issue depends on this issue. label Jan 27, 2022
@originalfoo
Copy link
Member Author

originalfoo commented Jan 27, 2022

I've checked through each option via visual inspection, then tested each of them in game, which is something I never want to do again lol.

EDIT: The fact that turning option on often enables one or more other options (eg. enabling mod features) was an absolute freaking nightmare in respect to testing what was happening.

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 👍

@originalfoo
Copy link
Member Author

Ok to put this in to 11.6.4.3 release?

@krzychu124
Copy link
Member

I didn't test it but also didn't spot any bug (analyzed changes carefully by counting indexes on my fingers) 😄

@originalfoo
Copy link
Member Author

yeha, trust me, you don't want to try testing every option with the way the options screen currently works (especially Policies tab) I thought my brain was going to burst lol

save[31] = (byte)(Options.parkingRestrictionsOverlay ? 1 : 0);
save[32] = (byte)(Options.banRegularTrafficOnBusLanes ? 1 : 0);
save[33] = (byte)(Options.showPathFindStats ? 1 : 0);
save[34] = Options.altLaneSelectionRatio;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Loads DLSPercentage, and stores altLaneSelectionRatio, hope they are the same variable

Copy link
Member

Choose a reason for hiding this comment

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

yup
image

@originalfoo originalfoo modified the milestones: 11.6.5, 11.6.4-hotfix-3 Jan 29, 2022
@originalfoo originalfoo merged commit c8f97ef into master Jan 29, 2022
@originalfoo originalfoo deleted the refactor-optionsmanager branch January 29, 2022 04:16
originalfoo added a commit that referenced this pull request Feb 1, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants