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

Move position into Creature #50621

Merged
merged 1 commit into from
Aug 9, 2021
Merged

Conversation

eltank
Copy link
Contributor

@eltank eltank commented Aug 8, 2021

Summary

Infrastructure "Move position into Creature"

Purpose of change

The position field and related getters and setters are duplicated in Character and monster, the 2 subclasses of Creature.

Describe the solution

Remove the duplication by moving the code into creature.h. Also position is now protected.
Monster, Character and npc still override setpos() in order to do other stuff, but now they call the superclass' setpos() instead of modifying the position directly.

Describe alternatives you've considered

Going one step further by making position private. Would need to fix many references to use pos() instead.

Testing

For the save/load code: loaded an existing save made prior to the change, got no errors. Spawned a monster and an NPC, saved and loaded again to verify they didn't teleport.

Additional context

@BrettDong BrettDong added Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style [C++] Changes (can be) made in C++. Previously named `Code` labels Aug 9, 2021
@eltank
Copy link
Contributor Author

eltank commented Aug 9, 2021

Forgot to update the save/load code, done now.

@anothersimulacrum
Copy link
Member

#42291

@eltank
Copy link
Contributor Author

eltank commented Aug 9, 2021

#42291

I am aware of the location class. Are you suggesting moving the implementation there? I got the impression that it's meant to be a pure virtual class.

@anothersimulacrum
Copy link
Member

Sorry, I should have left a comment, just wanted to make you aware of it if relevant/link the PRs together.

@kevingranade kevingranade merged commit 07570f7 into CleverRaven:master Aug 9, 2021
@eltank eltank deleted the creature_pos branch August 9, 2021 06:24
satheon49 pushed a commit to satheon49/Cataclysm-DDA that referenced this pull request Aug 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants