1. Hercules bug tracker archive
  2. Hercules Bug Archives
  3. Core
  4. Pc Bonus Vars 'zeroing'

Pc Bonus Vars 'zeroing'

Issue information

Issue ID
#858
Status
Fixed
Severity
Low
Started
Hercules Elf Bot
Jan 22, 2008 3:00
Last Post
Hercules Elf Bot
Jun 24, 2012 1:30
Confirmation
N/A

Hercules Elf Bot - Jan 22, 2008 3:00

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

I always thought the way eathena does player item bonus variables zeroing was a joke, but only now did I find out how bad it really is...

Here are the key points:
  • Arrays and variables, separated into long blocks. Some are zeroed, some are not.
    The exact ordering of entries must be preserved otherwise everything screws up.
  • Usage of memset(), with size equal to a sum of a multiple-page-long list of individual variable sizeof()'s.
    Each new variable needs to be manually added here, otherwise it won't get zeroed.
  • The memset() calls are keyed by addresses of some delimiting variables. Place your new var in an inappropriate place, and it messes up.
...
Wouldn't it be easier to just zero out the entire thing, then re-add values that are supposed to be nonzero? It's pages and pages of error-prone C code, can't this be done in some more intelligent way?

This post has been edited by theultramage: Jan 21 2008, 07:01 PM

Hercules Elf Bot - Dec 17, 2011 6:05

Originally posted by [b]Ind[/b]
I think skotlex suggestion back in the original report would be a nice approach[code]
memset (&sd->bonus, 0, sizeof(sd->bonus));[/code]

Hercules Elf Bot - Dec 17, 2011 9:55

Originally posted by [b]Epoque[/b]
In all honesty, I'd like to see some areas of the [i]struct map_session_data;[/i] object being re-written or re-organized. The whole thing is one big lump of confused and mixed-up values.

IE:
[code]unsigned int canlog_tick;
unsigned int canuseitem_tick; // [Skotlex]
unsigned int canusecashfood_tick;
unsigned int canequip_tick; // [Inkfish]
unsigned int cantalk_tick;
unsigned int canskill_tick; // used to prevent abuse from no-delay ACT files
unsigned int cansendmail_tick; // [Mail System Flood Protection]
unsigned int ks_floodprotect_tick; // [Kill Steal Protection][/code]

Could be condensed down to:
[code]struct {
unsigned int logout;
unsigned int item;
unsigned int cashfood;
unsigned int equip;
unsigned int talk;
unsigned int skill;
unsigned int mail;
unsigned int ks_floodprotect;
} tick;[/code]

That's only an example, there's likely to be a better way to write that block, however the point I'm making is that storing these values in an inline struct makes no difference to performance, it still points to the same memory address. This is definitely something worth looking into.

I'd also consider removing a lot of the static types in the structure, such as 'unsigned int' for time tick values, replacing them with system specific 'time_t' values (I believe some operating systems handle this type differently (ulong vs. uint) and it also adds structure/definition to the entire file header).

This post has been edited by Epoque on Dec 17, 2011 10:34

Hercules Elf Bot - Jun 24, 2012 1:30

Originally posted by [b]Ind[/b]
fixed some commits ago