[Freeciv-Dev] (PR#12706) Events framework

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=12706 >

Some thoughts (including recording some things I already said on IRC):

I think the design is good.  However there are some crucial features
that must work before committing it:

1.  I18n.  There's code for this but it needs testing.

2.  Save and restore of globals.  Naturally the globals must be saved in
the savegame.  There's already a mechanism for this (api_store); I
assume it's untested.  However it looks like it requires the
store/recover functions to be called separately in each lua function
where the global functions are used.  This is unacceptable; it will make
even a trivial scenario like a tutorial very complicated.  Ideally the
save+restore would be done automatically.  Failing that, there needs to
be just one place where there is code to save and load.  One possible
way is to call a register_global_variable() function in the main part of
the script; then for all registered globals we do something-or-other to
serialize and deserialize them.  Another (worse) possibility is to
provide a save() and load() signals which the script can hook into to do
the serialization and deserialization by hand (using the existing store
API).

3.  Events in scenarios.  For a tutorial scenario it should be a
*scenario*, not a new ruleset.  Thus a separate bit of lua in the .sav
needs to be read in and executed.  Note this same bit must also be saved
again when the game is saved.

Now a few other unrelated comments...

1.  You called the directory "script".  There is already a root
directory "scripts", which just holds some (currently one) utility
scripts.  Maybe the new directory should be called "scripting"?

2.  I still don't like that every header file is called api_*.  It's
good that the interface is modular but I still think it is redundant to
call any header file "api".  Could you just drop the api_ bits from each
filename?  (Perhaps the most annoying part is that it makes
autocompletion and directory sorting harder ;-).)

I think that's it for now.  I didn't look at all of the api_* files though.

-jason