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

Remove more uses of AList/AListElem #218

Merged
merged 4 commits into from
Sep 28, 2024
Merged

Remove more uses of AList/AListElem #218

merged 4 commits into from
Sep 28, 2024

Conversation

jt-traub
Copy link
Contributor

  • Shields are now an stl container. Reimplemented get highest shield in terms of the STL in-place since it was only used once and got rid of the shieldlist class as well. In order to simplify memory management replaces the Shield * with shared_ptr which provides a smart ref-counted pointer, so that just clearing (or erasing) an element from the container will delete the allocated memory. Moved the definition of Shield into army.h which was the only thing that used it now.
  • Removed all uses of AList from the quests system (quests and rewards). Retained the QuestList class since it had a number of useful functions and reworked it using an internal std::vector. Renamed all the member functions to follow intended style going forward.
  • Removed vestiges of AList from productions. This had mostly been done, so this was just cleaning up a leftover place or two where it hadn't been fully removed. Also renamed the production class's functions using the new style.
  • Removed vestiges of AList from markets. This has been partially done previously, this was just cleaning up some leftover turds. Renamed Market class functions to use new style of naming. Made the enum type for the market into a named enum and part of the Market class for scoping reasons.

Shields are now an stl container.  Reimplemented get highest shield
in terms of the STL in-place since it was only used once and got rid
of the shieldlist class as well.  In order to simplify memory management
replaces the Shield * with shared_ptr<Shield> which provides a smart
ref-counted pointer, so that just clearing (or erasing) an element from
the container will delete the allocated memory.
Removed all uses of AList from the quests system (quests and rewards).
Retained the QuestList class since it had a number of useful functions
and reworked it using an internal std::vector.  Renamed all the
member functions to follow intended style going forward.
This had mostly been done, so this was just cleaning up a leftover
place or two where it hadn't been fully removed.  Also renamed the
production class's functions using the new style.
@@ -28,26 +28,7 @@
#include "gameio.h"
#include "gamedata.h"

Market::Market()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved these into the header file with the more modern style of member variable initialization.

else {
if (population >= maxpop)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just denesting some ifs.

//AString Report();
};

class MarketList : std::vector<Market*> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This wasn't used, AND deriving from STL containers is a bad idea since they don't have virtual destructors.

This has been partially done previously, this was just cleaning up
some leftover turds.  Renamed Market class functions to use new
style of naming.  Make the enum type for the market into a named
enum.
Copy link
Contributor

@valdisz valdisz left a comment

Choose a reason for hiding this comment

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

lgtm

@jt-traub jt-traub merged commit 31ee961 into master Sep 28, 2024
10 checks passed
@jt-traub jt-traub deleted the jt-no-more-alist-3 branch September 28, 2024 17:36
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