1. Hercules bug tracker archive
  2. eA Bug Archives
  3. Possible optimization on skill.c

Possible optimization on skill.c

Issue information

Issue ID
#4036
Status
Working as Intended
Severity
Low
Started
Hercules Elf Bot
Feb 3, 2010 11:07
Last Post
Hercules Elf Bot
Apr 18, 2012 9:37
Confirmation
Yes (2)
No (0)

Hercules Elf Bot - Feb 3, 2010 11:07

Originally posted by [b]Kazukin[/b]
http://www.eathena.ws/board/index.php?autocom=bugtracker&showbug=4036

While I was working on a mod to expand skill_require_db to allow alternative requirements for skills I found that 'Potion/Berserk Pitcher (AM_POTIONPITCHER, AM_BERSERKPITCHER)' skills and 'Slim Potion Pitcher (CR_SLIMPITCHER)' skill bypassed my mod on skill_get_requirement because they got the item requirements directly using 'skill_db[skillid].itemid[x]' inside skill_castend_nodamage_id and skill_castend_pos2. Maybe these skills' requirements should be obtained by a call to skill_get_requirement inside these functions as it is usually done.

skill.c
http://pastebin.com/f697715cb

Hercules Elf Bot - Feb 20, 2012 7:23

Originally posted by [b]Lighta[/b]
Up, since this consistency wasn't done, that sp alteration wont be effective for the skill he listed :
[code]
if( sc ) {
if( sc->data[SC__LAZINESS] )
req.sp += req.sp + sc->data[SC__LAZINESS]->val1 * 10;
if( sc->data[SC_RECOGNIZEDSPELL] )
req.sp += req.sp / 4;
}
[/code]
That may still sound low, I'm just updating and it's really a quick change.

Hercules Elf Bot - Mar 18, 2012 23:06

Originally posted by [b]Epoque[/b]
If this change was implemented, however, and Potion Pitcher, Berserk Pitcher or Slim Potion Pitcher were used through a skill-scroll, it would not return any of the requirements. Explicitly:

[code]if( sd->skillitem == skill )
return req; // Item skills and Hocus-Pocus don't have requirements.[Inkfish][/code]

Plus who knows what other codes would interfere with the process. It's best left as it is for the time being, albeit a silly piece of code, it's secure and doesn't require remodelling other functions to make it work.

This post has been edited by Epoque on Mar 18, 2012 23:06

Hercules Elf Bot - Mar 28, 2012 3:50

Originally posted by [b]Lighta[/b]
Are you saying will keep something incoherent just to not break itemscroll ?
I understand this could have more impact that he mention and require more investigation but common was you talking about solidify ea base ? This is a perfect exemple of incoherence that need to be changed for easier maintenance in future so please don't mark it as WAI just cause you don't feel he worth investigation atm.

Hercules Elf Bot - Apr 2, 2012 20:52

Originally posted by [b]Epoque[/b]
[quote name='Lighta' timestamp='1332906615' post='8064']
Are you saying will keep something incoherent just to not break itemscroll ?I understand this could have more impact that he mention and require more investigation but common was you talking about solidify ea base ? This is a perfect exemple of incoherence that need to be changed for easier maintenance in future so please don't mark it as WAI just cause you don't feel he worth investigation atm.
[/quote]

The thing is, is that it's not worth the effort investigating right now. We have some pretty large systems/fixes to develop and big changes (such as skill processing, skill order of processing etc.) comes later once we've fixed up the majority of bugs and optimised where we can.

Like I said in the last post, the actual code functions as intended. The renovation would probably be more aesthetic than anything else. Sure, it could perhaps do with a bit of re-wording, perhaps moving around a little bit, but when we decide to get to those kinds of commits we'll focus on them instead.