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

Decruft citizen status #837

Merged
merged 23 commits into from
Apr 13, 2020
Merged

Decruft citizen status #837

merged 23 commits into from
Apr 13, 2020

Conversation

originalfoo
Copy link
Member

While looking at ExtCitizenInstanceManager.cs I noticed the methods that output localized citizen status (that gets shown on citizen world info panel) for residents and tourists was full of duplicate and extraneous code. This PR cleans it up - a lot.

The target was being defined per status, which was adding lots of useless lines to read through. Now it's only defined once at start of the methods.
No point defining a load of local variables if we're not going to use them; check we're not confused before doing that stuff.
Make referring to citizen more concise.
For consistency with other id vars.
Used to determine if citizen is caught in a flood or tornado.
Incorporate IsSweptAway() and condense confused statuses

Remove duplicate code
Determines if a vehicle is owned by the specified citizen.
Incorporate IsVehicleOwnedByCitizen()
Checks if a building id is an outside connection.
Incorporate IsOutsideConnection() method
Comment what targetIsNode means
We want to refer to vehicle and it's info within the switch, not vehicle manager.
We are interested in vehicle and its info, not vehicle manager.
It's almost always true, so set true by default then we only need to set to false where applicable.
@originalfoo originalfoo added the code cleanup Refactor code, remove old code, improve maintainability label Apr 13, 2020
@originalfoo originalfoo self-assigned this Apr 13, 2020
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.

Nice

@originalfoo originalfoo merged commit 5116893 into master Apr 13, 2020
@originalfoo originalfoo deleted the decruft-citizen-status branch April 13, 2020 21:36
@originalfoo originalfoo added this to the 11.3.2 milestone Apr 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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