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 vocations: Reload, extracted into tfs::game::vocations namespace and code improvements #4807

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

ramon-bernardo
Copy link
Contributor

@ramon-bernardo ramon-bernardo commented Oct 8, 2024

Pull Request Prelude

Changes Proposed

  • Namespace: Introduced tfs::game::vocations namespace to organize vocation-related logic in a more structured way.
  • Smart pointers: Transitioned from raw pointers to std::shared_ptr for improved memory management and safety.
  • Loading vocations: Implemented load_from_xml(...) to load vocation data from an XML file, with an option to reload existing data.
  • Searching vocations:
    • Renamed functions for finding vocations by ID and name:
      • find_by_id(uint16_t id)
      • find_by_name(std::string_view name)
      • find_by_promoted_id(uint16_t id)

Note: It is not possible to delete a vocation at runtime; only to modify or add new ones.

Before reload.
image

After changed and reload.
image

Usage

// Load vocation data from XML (reload set to false).
std::ifstream is{"data/XML/vocations.xml"};
tfs::game::vocations::load_from_xml(is, "data/XML/vocations.xml");

// Find a vocation by ID and check if it's valid.
auto vocation = tfs::game::vocations::find_by_id(1);
if (vocation) {
    std::cout << "Found vocation: " << vocation->name << std::endl;
}

// Find a vocation by name.
auto knight = tfs::game::vocations::find_by_name("Knight");
if (knight) {
    std::cout << "Found vocation: " << knight->name << std::endl;
}

// Check skill tries and mana requirements for a specific vocation.
auto skill_tries = vocation->getReqSkillTries(SKILL_SWORD, 20);
auto mana_req = vocation->getReqMana(5);

// Remove vocations or reload them from XML.
std::ifstream is{"data/XML/vocations.xml"};
tfs::game::vocations::load_from_xml(is, "data/XML/vocations.xml", true);

src/vocation.cpp Fixed Show fixed Hide fixed
@ramon-bernardo
Copy link
Contributor Author

The future idea is to move most or all of Vocation to lua, removing it from the core

@ramon-bernardo ramon-bernardo marked this pull request as ready for review October 9, 2024 00:36
return 1600 * std::pow(manaMultiplier, static_cast<int32_t>(magLevel - 1));
}

bool tfs::game::vocations::load_from_xml(std::istream& is, std::string_view filename, bool reload)

Check warning

Code scanning / CodeQL

Poorly documented large function Warning

Poorly documented function: fewer than 2% comments for a function of 112 lines.
@EPuncker EPuncker requested a review from a team October 9, 2024 00:41
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