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

Battle & skill code cleanup #73

Open
gepard-me opened this issue Jul 26, 2013 · 5 comments
Open

Battle & skill code cleanup #73

gepard-me opened this issue Jul 26, 2013 · 5 comments
Labels
component:core Affecting the Hercules core (i.e. not the game mechanics directly) component:databases Affecting the databases available in the db/ folder status:confirmed Issue is valid and can be reproduced type:enhancement Issue describes an enhancement or feature that should be implemented
Milestone

Comments

@gepard-me
Copy link
Member

This is something I've been thinking about for long time now.

  • proper separation of battle and skill code into their own modules (eg int battle_calc_skillratio is placed in battle.c now, instead of skill.c)
  • splitting code into smaller functions responsible of performing exactly one task (ideally) for improved readability and reusability of code
  • moving as many hardcoded numbers and flags into skill_db*.txt files (eg already mentioned skill damage ratios, certain skills that bypass X, etc)
  • removing awful switch (skill_id) in favor of expanding internal skill database representation with function pointers to perform extra actions unique for given skill id (eg. overriding default skill damage ratio calculation, adding extra conditions or extra effects that would be too complex to represent in plain txt db files); in future allow overriding these functions by plugins for easy skill modifications

Opinions, suggestions?

@shennetsind
Copy link
Member

  • I think it is much necessary
  • not sure what exactly you mean (anything on your mind you could use as a example?)
  • yeey o-o question would this be added in the current format or you intend on libconfig'ing it?
  • sounds awesome

@gepard-me
Copy link
Member Author

About no.2
I mean extracting parts of huge functions responsible for a specific task within that function, especially if they can be found in other places. For example battle_calc_weapon_attack, battle_calc_magic_attack and battle_calc_misc_attack share some common lines (eg determining attack element) that could and should be moved to separate function.

@MishimaHaruna
Copy link
Member

In no.4, how would those function pointers be assigned to? How would you make it know i.e. that the function extra_effects_for_the_foo_skill() belongs to the skill NV_FOO when loading the skill database, thus it must be assigned to its entry?

@gepard-me
Copy link
Member Author

I didn't consider moving that particular information to the database yet. So they would be assigned in some sort of load_default_extra_effects(). Consider, for example, a function to calculate skill damage ratio. For most skills this would be set to a function that fetches appropriate value from skill_db (default approach), but for several skills with unique damage ratios, pointers would be initialized to unique functions. I don't see much problem with that approach though. I didn't plan to make it customizable with db file only; it looks to me more like a job for plugin.

@MishimaHaruna
Copy link
Member

Okay, sounds good then. I was worried about excessive amounts of symbols requiring to be defined, in case you wanted to make them assignable (i.e. by function name) directly from the skill_db.

@MishimaHaruna MishimaHaruna added this to the Architecture milestone Jun 7, 2015
@MishimaHaruna MishimaHaruna added the status:confirmed Issue is valid and can be reproduced label Sep 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:core Affecting the Hercules core (i.e. not the game mechanics directly) component:databases Affecting the databases available in the db/ folder status:confirmed Issue is valid and can be reproduced type:enhancement Issue describes an enhancement or feature that should be implemented
Projects
None yet
Development

No branches or pull requests

3 participants