Issue Information
-
#000342
-
3 - Medium
-
Confirmed
Issue Confirmations
-
Yes (0)No (0)
0
Originally posted by theultramage
http://www.eathena.w...ker&showbug=342
(this is a TODO, related topic: #279)
The WoE system is a heap of unreadable spaghetti (IMG:style_emoticons/default/sleep.gif)
If you look at it closer, the entire guardian_data structure is totally redundant - all info could be extracted from a guild lookup (using guild_id). And even that wouldn't be needed because you could just look up castle info (and thus the owning guild) for the map the guardian is standing on.
This, of course, for an expense of an O(log n) lookup, where n is the ammount of guilds (a small number) or castles (a very small number). Instead, there are pages and pages of code dedicated to maintaining this 'cache' and syncing it whenever something changes.
Now to the db part. My proposal is to completely remove any mention of guardians from the castles table. Why?
The `visibleG?` columns keep track of which guardians are hired and which aren't. This in turn forces the gw script to make castle information requests from the server. If these were script variables instead, the whole mess could be removed.
The `gHP?` columns track how much current hp each guardian has. What the?! was my reaction when I realized this. To sum it up:
Some whacko decided that 'current' guardian hp values should be made persistent. So he adds it to the castle's data, and then adds hacks to spawn the mob with non-full hp. Why the hell would you want to make guardian hp persist between server restarts (only valid reason), when a priest can just go around and heal all guardians to full hp? Why give special preference to guardian mobs, which can be killed in just 10 seconds?
PS: some more minor junk to remove (this one's from jA):
The Emperium has a npc::OnAgitBreak death event wired in. Code says that for each such mob, guild_agit_break(md) is called. This function just starts a battle_config.gvg_eliminate_time timer, targeting the function guild_gvg_eliminate_timer(). This function just replaces the 'Break' part of the event name with 'Eliminate' and runs the script. Nothing more. So basically the code is doing what the script engine should be doing. The script engine can even look up the config setting using getbattleflag().
So here you go, plenty of material for code removal.
This post has been edited by theultramage: Nov 1 2007, 06:43 AM
http://www.eathena.w...ker&showbug=342
(this is a TODO, related topic: #279)
The WoE system is a heap of unreadable spaghetti (IMG:style_emoticons/default/sleep.gif)
If you look at it closer, the entire guardian_data structure is totally redundant - all info could be extracted from a guild lookup (using guild_id). And even that wouldn't be needed because you could just look up castle info (and thus the owning guild) for the map the guardian is standing on.
This, of course, for an expense of an O(log n) lookup, where n is the ammount of guilds (a small number) or castles (a very small number). Instead, there are pages and pages of code dedicated to maintaining this 'cache' and syncing it whenever something changes.
Now to the db part. My proposal is to completely remove any mention of guardians from the castles table. Why?
The `visibleG?` columns keep track of which guardians are hired and which aren't. This in turn forces the gw script to make castle information requests from the server. If these were script variables instead, the whole mess could be removed.
The `gHP?` columns track how much current hp each guardian has. What the?! was my reaction when I realized this. To sum it up:
Some whacko decided that 'current' guardian hp values should be made persistent. So he adds it to the castle's data, and then adds hacks to spawn the mob with non-full hp. Why the hell would you want to make guardian hp persist between server restarts (only valid reason), when a priest can just go around and heal all guardians to full hp? Why give special preference to guardian mobs, which can be killed in just 10 seconds?
PS: some more minor junk to remove (this one's from jA):
The Emperium has a npc::OnAgitBreak death event wired in. Code says that for each such mob, guild_agit_break(md) is called. This function just starts a battle_config.gvg_eliminate_time timer, targeting the function guild_gvg_eliminate_timer(). This function just replaces the 'Break' part of the event name with 'Eliminate' and runs the script. Nothing more. So basically the code is doing what the script engine should be doing. The script engine can even look up the config setting using getbattleflag().
So here you go, plenty of material for code removal.
This post has been edited by theultramage: Nov 1 2007, 06:43 AM
