Various Script Config Settings Removal

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

I mentioned this before, but not sure where, so I'm saying it again.

script_event_type removal
Mode 0 is ridiculous. It forces you to put ALL of an event's code into a single monolithic npc, which is bad design. It also makes various scripts totally incompatible with each other, and also causes a configuration chaos (until people realize '1' is the right setting).

Related changesets:
pre-svn - the custom monolithic event implementation
1116 - "Added some new script event related options to script config [celest]"
r1282 - "Implemented 'event_script_type' [celest]"
r6494 - "Added a npc-script-event cache to avoid looking up event-scripts every time they need to be executed [skotlex]"
Side-note: jAthena actually imported this thing (as their r1792), but only the 'new' method, as it was clear that the old is inferior.

And while we're at it...
event_requires_trigger removal
Since r914, there's a mechanism that's ON by default, that forces you to store permanent character variables to indicate whether a particular OnPC* event may execute or not.

So, instead of running the script and stopping on an 'if' at the beginning, this thing lets you do an 'if' before running the script, at the expense of lugging around extra state data and acccessing script variables from inside the source code.
This is basically equivalent to "doing 'if' before a function call instead of doing 'return false' inside the function", just that the function call is more expensive. To me, this screams 'premature optimization'... plus it makes it rather ugly if you want to write a conformant eathena script. Or a conformant athena script, for that matter.
Side-note: jAthena has a per-event global config setting instead of per-player. Still doesn't matter much because as I described above, this whole thing's purpose is purely for a (negligible) performance improvement.

Also...
*_event_name removal
Since r1116, the On- event names are no longer hardcoded, instead they are exported into the config file and thus are parametric. While this might sound like a good idea at first, realize that for script interoperability, these names MUST STAY THE SAME everywhere...

They could be moved to a const char[] at the beginning of script.h or something, or inlined back into the code. Note that stuff like PC_DIE_COUNTER or SG_HATE_VAR are also script-related things that are hardcoded - so a little bit of consistency wouldn't hurt.
Note: jAthena imported these without the 'Event' part in the name.

loadevent removal?
Whenever a char changes maps, a PCLoadMap event is supposed to be executed for all server npcs. I have 2 issues with this.
In r7358, Skotlex added a special rule. Changing maps will only trigger the event if the destination map is flagged as 'loadevent'.
Notice that this doesn't mean "triggers all npcs on the destination map", but "all npcs". Even on other maps... Turning this into just another speed optimization thing. I'm pointing out that a server will only have a few of such npcs, and the npc event index is there to quickly enumerate all of them.
Iin some cases this 'optimization' might actually hinder scripters from doing what they want - for example, now it's impossible to write a script that tracks you across ALL maps, without mapflagging each and every one of them.

Additionally, in r7325, Lance added a @maploaded$ variable to be set during a mapchange. I find this totally useless, as getmapxy(.@map$) does the job already and it doesn't always create a persistent variable.

This post has been edited by theultramage: Dec 3 2007, 09:45 PM