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

Add skill_db2.conf and some SkillDB code fix/simplification #3216

Merged
merged 9 commits into from
Jul 13, 2023

Conversation

guilherme-gm
Copy link
Member

@guilherme-gm guilherme-gm commented Jun 18, 2023

Pull Request Prelude

Changes Proposed

Add skill_db2.conf

This follows the same idea of the existing item_db2.conf / mob_db2.conf, allowing users to create skill_db entries (or override existing ones).

Just like the item/mob dbs, it also includes the Inherit field to allow changing only parts of a skill.

Code simplification

This is a suggestion to reduce the amount of if/else's in the flags parsing. Instead replacing them with an array of key/values.

I applied it to the functions related to skill_db.conf that had a long list and were only doing that. There is a few other cases with smaller lists which I skipped.

I have used this small diff to validate if the changes did not cause any side effect:
compare_skills.diff.txt

If you apply it to master, build and run, you will get a list of skill flags. Save it. Change to this branch and do the same thing. Both should present the same output.

Case enforcing and fix

During the simplification I also changed the comparison from strcmpi to strcmp. For our base code, it only triggered issues with SameGuild flag (which was being used as Sameguild. Custom changes will trigger warnings for people to work and use the correct case.

Issues addressed:
None, I think

includes support for inheriting the original definition in order to change specific configs
src/map/skill.c Outdated Show resolved Hide resolved
- change case-insensitive check to case-sensitive (strcmpi -> strcmp)
  Database flags should always have their case taken into account, to
  this change will detect and alert about flags with the wrong case
- simplify the long if/else chain with the flag possibilities into an
  array of flag/value. This:
  - makes it easier to read/maintain
  - avoids typos/mistakes due to replicating the condition many times
- change case-insensitive check to case-sensitive (strcmpi -> strcmp)
  Database flags should always have their case taken into account, so
  this change will detect and alert about flags with the wrong case
- simplify the long if/else chain with the flag possibilities into an
  array of flag/value. This:
  - makes it easier to read/maintain
  - avoids typos/mistakes due to replicating the condition many times
- change case-insensitive check to case-sensitive (strcmpi -> strcmp)
  Database flags should always have their case taken into account, so
  this change will detect and alert about flags with the wrong case
- simplify the long if/else chain with the flag possibilities into an
  array of flag/value. This:
    - makes it easier to read/maintain
    - avoids typos/mistakes due to replicating the condition many times
- change case-insensitive check to case-sensitive (strcmpi -> strcmp)
  Database flags should always have their case taken into account, so
  this change will detect and alert about flags with the wrong case
- simplify the long if/else chain with the flag possibilities into an
  array of flag/value. This:
    - makes it easier to read/maintain
    - avoids typos/mistakes due to replicating the condition many times
- change case-insensitive check to case-sensitive (strcmpi -> strcmp)
  Database flags should always have their case taken into account, so
  this change will detect and alert about flags with the wrong case
- simplify the long if/else chain with the flag possibilities into an
  array of flag/value. This:
    - makes it easier to read/maintain
    - avoids typos/mistakes due to replicating the condition many times
- change case-insensitive check to case-sensitive (strcmpi -> strcmp)
  Database flags should always have their case taken into account, so
  this change will detect and alert about flags with the wrong case
- simplify the long if/else chain with the flag possibilities into an
  array of flag/value. This:
    - makes it easier to read/maintain
    - avoids typos/mistakes due to replicating the condition many times
- change case-insensitive check to case-sensitive (strcmpi -> strcmp)
  Database flags should always have their case taken into account, so
  this change will detect and alert about flags with the wrong case
- simplify the long if/else chain with the flag possibilities into an
  array of flag/value. This:
    - makes it easier to read/maintain
    - avoids typos/mistakes due to replicating the condition many times
- change case-insensitive check to case-sensitive (strcmpi -> strcmp)
  Database flags should always have their case taken into account, so
  this change will detect and alert about flags with the wrong case
- fixed case of "Sameguild" value in skill_db (should be "SameGuild")
- simplify the long if/else chain with the flag possibilities into an
  array of flag/value. This:
    - makes it easier to read/maintain
    - avoids typos/mistakes due to replicating the condition many times
@guilherme-gm guilherme-gm changed the title Add skill_db2.conf and some SkillDB code simplification Add skill_db2.conf and some SkillDB code fix/simplification Jun 19, 2023
@guilherme-gm guilherme-gm marked this pull request as ready for review June 19, 2023 00:33
@MishimaHaruna MishimaHaruna added this to the Release v2023.07.12 milestone Jul 12, 2023
@MishimaHaruna MishimaHaruna merged commit 36f1e7f into HerculesWS:master Jul 13, 2023
16 of 254 checks passed
@guilherme-gm guilherme-gm deleted the skilldb2 branch February 10, 2024 20:56
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.

None yet

2 participants