Interface Redesign

[AF]

You were going to add a new event for everything anyway weren't you though?

Eitherway the modular OO approach is generally considered good programming practice when used correctly. It principle just happens to shine when applied to AIs.

[hoijui]

a new event for everything.. if you mean a new function for every event, then yes, but then i would rather make each function receive the arguments directly, instead of the event object containing them.

i found out that its possible to define a base class in java in SWIG for certain wrapped classes or structs (eg based on the name of those).
i can define a base class which contains the function to return the void* and attach it to all classes and structs applying to "*Event". This means, that when a new event is added, i only have to execute SWIG, without modifying the interface file (SWIG config file). So i will be able to use the C interface quite directly with JAI, and i have the advantage that all the Events are automatically visible in Java as classes (in C or C++ one always has to look into the engines C interface header). This makes the C interface quite usable for JAI then.

[hoijui]

A draft using void* and Event structs.

Most things should soso be ok->compile, except the part which loads the shared lib and calls functions on it.

i still think functions woudl be better, but if i can manage to create some regexes which generate all the needed defines and Event structs relatively automatic, i may do the event thing if we once agree on a draft.

[zenzike]

Thanks for the draft.

I'm of the opinion that we shouldn't use the events system for the core engine functions that aren't in game events. My previous post suggested this:

Code: Select all

class SPRING_API IAIView {
 init();
 update();
 load(const* char fileName);
 save(const* char fileName);
 handleEvent(int eventID, void* event)
}

with the following events:

Code: Select all

Event
  UnitEvent
    UnitCreatedEvent
    UnitFinishedEvent
    UnitIdleEvent
    UnitDamagedEvent
    UnitDestroyedEvent
  EnemyEvent
    EnemyEnterLOSEvent
    EnemyLeaveLOSEvent
    EnemyEnterRadarEvent
    EnemyLeaveRadarEvent
    EnemyDamagedEvent
    EnemyDestroyedEvent
  MessageEvent

does everybody agree, or are there any objections? (and why?)

[hoijui]

how do you split what you call core engine functions from the rest?
non core engine functions are the ones that we can put into a logical structure?

I would prefer to either use only events or only functions. One reason for it: If something new gets added, and the interface contains partly functions and partly events, the person adding the new thing will just use whatever he likes more, and not what it would have to be with the logic we set up now.

[AF]

Stuff like init and update etc can be done via the handle message using a null pointer for void* as they have no parameters. No reason to mix the two styles up.

[zenzike]

Ah yes, good point. I was thinking of having to create some sort of empty struct, and that would be useless overhead. I totally agree, we should use a null event and use the handleEvent() as usual.

[hoijui]

an empty struct is a cleaner design. so we treat everything equally. also if we want to add an argument to something that had none before, we can just add it. The overhead in instantiating an empty struct is marginal and not worth moving away from a clean design.

[AF]

Why instantiate every time we send an event? Instead of sending the same type of struct each time (an empty struct) we can send the same struct each time (the same empty struct each and every time)

That way we only ever instantiate it once at the beginning of the program.

Of course this seems like one of those things that would end up on the dailyWTFs site.

[jcnossen]

Instantiating an empty struct on the stack costs nothing... This seems like redundant optimization, or not even optimization at all.

[hoijui]

gracias jc!

[Tobi]

Before continuing reading, consider that there are 2 different issues being discussed at the same time.

• One is whether one function per event should be used or whether one case label per event should be used.
• The other is how the arguments to the event should be passed.

Function or HandleEvent?

The problem with functions as opposed to a HandleMessage function is exaggerated big time in this thread.

More specifically, with a C ABI, functions have, AFAICS, no disadvantages for the interface compared to a single HandleEvent function.

Lets summarize the options in code snippets: (parameter passing left out cause it's different subject)

HandleMessage:

Code: Select all

void HandleEvent(void* ai, int id, /* ... */) {
  switch (id) {
     case ENEMY_ENTERED_LOS_EVENT:
        // ...
        break;
     case ENEMY_LEFT_LOS_EVENT:
        // ...
        break;
     // ...
  }
}

Functions:

Code: Select all

void EnemyEnteredLos(void* ai, int id, /* ... */) {
  // ...
}
void EnemyLeftLos(void* ai, int id, /* ... */) {
  // ...
}

The entire problem with the current interface is that if a single function is added, the AI must be recompiled, because the interface classes' vtable changes, and the vtable's format is pretty much undefined (read no standard ABI). With C ABI, there is no undefined-format vtable, so this problem goes away magically.

With simple C functions however, it is trivial to put a pattern in spring code like this, so events are entirely opt-in from the AI side, just like with HandleEvent approach:

Code: Select all

  // on loading the AI:
  EnemyEnteredLosEvent = (ENEMYENTEREDLOSEVENT)GetFunctionPointer(ai_dll_handle, "EnemyEnteredLosEvent);
  // when calling the event in the engine:
  if (EnemyEnteredLosEvent != NULL) // only call event if AI handles it
    EnemyEnteredLosEvent(...);

(left parameter passing details away for brevity)

Note also how similar this is to the standard pattern to call events in .NET:

Code: Select all

  public event EventHandler<EventArgs> EnemyEnteredLos;
  void OnEnemyEnteredLos(EventArgs e) {
    if (EnemyEnteredLos != null)
      EnemyEnteredLos(this, e);
  }

This is a good thing IMHO because people who have done .NET development will immediately recognize the pattern.

The only minor disadvantage of functions I do see however, is that it is slightly more code on the engine side.

With regards to binary compatibility, about which this thread seems to be mostly, adding an exported function to an AI does not break binary compat, so a newer AI can always be used in an older Spring version. (forward compatibility!)

Adding an extra event does also not break binary compatibility (as long as the engine doesn't unconditionally call it, ie. the conditional in above code sample must be present), so an older AI can always be used in a newer Spring version. (backward compatibility!)

tl;dr: The choice between HandleEvent approach and multiple functions approach when doing a C interface (NOT C++) is arbitrary, and neither of the approaches has significant advantages over the other.



Parameter passing

Parameter passing isn't problematic in any way when we disregard compatibility. However, when we introduce it does become a problem, as you want to prevent breaking interfaces all the time by changing parameters passed to an event.

For brevity, I will assume that any other modification on a parameter list apart from adding a new parameter on the end will break binary compatibility for this event. Therefore such modifications do not have to be considered. (There are ways to attack them but it's outside scope of this post.)

The two options for parameter passing then are:

• passing them as regular function arguments, and
• passing a pointer to a structure/union containing the arguments.

The choice here is much easier. Passing parameters as regular function arguments is already impossible if you use HandleEvent approach in first part of post (read earlier posts about evilness of varargs).

When using functions, parameter passing is ok-ish until you start adding arguments. Only with some calling conventions one can safely add extra arguments to an argument list without breaking compat (the ones that let caller pop stack (as opposed to callee)). Because this gets way too technical, it's a bad solution

The other options is passing a pointer to a structure. Because structures/unions are always laid out identically in memory, provided you set right alignment setting, they provide a much stronger basis for ABI compatibility.

So here the choice is easy ... use structures for passing arguments if you ever want to add new arguments to existing events, even if you use regular functions to handle the events!

Only downside of structures is the setup/teardown anti pattern thats needed engine side to call the events (well without teardown usually, I assume). With some smart #ifdef..#endif'ing constructors in them this can be overcome easily though:

Code: Select all

struct EnemyEnteredLosEventArgs {
#ifdef __cplusplus
  EnemyEnteredLosEventArgs(int unit_id) : unit_id(unit_id) {}
#endif
  int unit_id;
};

  // this Spring internal function calls the AI-side event, whether it's HandleEvent() or EnemyEnteredLos()
  OnEnemyEnteredLos(EnemyEnteredLosEventArgs(unit_id));

tl;dr: use structures to pass parameters



Hope this clears some stuff up

[Tobi]

As a follow up, I figured one minor advantage of HandleEvent approach is that you can put exception handling in a single place (around the switch statement). A minor performance disadvantage with HandleEvents is the switch statement. The reduction in code duplication for exception handling tips the balance in favor of HandleEvents approach though, IMHO, especially since this C interface isn't ment to be used directly by AI devs anyway. (Multiple functions approach is better readable IMHO.)

(since exceptions won't propagate over C ABI boundary you have to either handle them, or never throw them at all )

[hoijui]

WOW!
thanks for that tobi!

i still think that functions have the advantage that the C interface is better understandable/readable and easier to use. and it will be used for creating other language interfaces eg for Java, so it has some importance. it is easier with functions cause i get the right struct, and do not have to know which eventTopic relates to which struct and convert a void* to that struct of the right type. sure, its a relatively small advantage, but as you said, that are most of the arguments we use here.

[hoijui]

..sooo?
i guess it is clear that we use structs, but not whether we use HandleEvent() or individual functions, right?

as i see it, if we use HandleEvent(), it is relatively easy to make a very simple C file on the AI side, which would pass events to individual functions.

if we use individual functions for the event structs, it is also easy to pass those to a single HandleEvent() method on the AI side. if one prefers the HandleEvent() approach.

the second method would require all the defines (indices and strings that are attached to/refer to events) on the AI side, while the first one would require them on the engine side of the interface.

in the end, it depends on what way would be used more. the C++ interface wrapper can be done with both ways equally simple, i would say. for Java it would be easier to use the functions approach, and i would guess that for most other languages too.

how are we going to decide? are we going to vote?
it seems as if there are no more arguments, as there were few posts the last two days here.

[hoijui]

a switch statement works the same way as if ... else if .. else if ...
right? this could cost a lot of time if we have, lets say... 300 events, and some at the end are very frequent.
we would not need a switch statement if we use simple functions.

[Tobi]

Switch statements can be optimized by the compiler using jump tables sometimes.

If else if else chains have problem of running into compiler limits. (VC has some max nesting level of 128 or so that is reached easily when you put 128 if-else blocks after each other )

Always use switch if it's possible, if else chaining is nice only if you have conditions that can't be represented in a switch (ie. checking for ranges, comparing strings, etc.)

[hoijui]

ok...
well.. after the preprocessor, our switch statement would look somehow like this:

Code: Select all

switch (messageTopic) {
   case 0: ... break;
   case 1: ... break;
   case 2: ... break;
   case 7: ... break;
   case 8: ... break;
   case 9: ... break;
}

* the lower we get, in the statement, the higher the numbers. some numbers will not appear, as some events go only Engine -> AI or vise versa.
i guess this will be optimized, right?

[zenzike]

If we use a switch statement we should use constants that are assigned to specific values.

Code: Select all

switch(eventID) {
  case INIT_EVENT: break;
  case UPDATE_EVENT: break;
  case UNIT_CREATED_EVENT: break;
  case UNIT_IDLE_EVENT: break;
  ...
}

And yes, the compiler will take care of optimisation, we won't have to worry. It's better to use constants rather than numbers, since it makes our intentions clearer to read.

On a completely different note, there has been a lot of talk about how the engine passes event information to the AI, whether through functions or the handleEvent() mechanism which will either be push, pull or a combination. I think we've left the issue of push/pull methods undiscussed since the choice seems obvious, but I think that there is a place for talking about this now.

Let's look at the difference between (pure) push and pull methods in the example of some event, for example, the UnitIdleEvent.

Push
If we're pushing the information about the idle unit, we'll need to send the UnitDef struct in the event itself. Using functions, this would just be one of the parameters, like:

Code: Select all

UnitIdle(int unitID, UnitDef* unitDef);

and in the events struct:

Code: Select all

struct UnitIdleEvent {
  int unitID;
  UnitDef* unitDef;
}

Advantage:
The advantage of using a data push is that if the information is required, the AI has it to hand, and doesn't need to go fishing for more. This means that there is no need for a data retrieval callback.
Disadvantage:
Not all the data is always required, and so we're actually just pushing information that isn't required. I also believe there's an implementation issue when it comes to nested structs in library interfaces (though I might be wrong about this).

Pull
In a pure pull implementation, we would get something like:

Code: Select all

UnitIdle()

or a handleEvent() call with a null struct. (In a pure pull implementation the handleEvent() way of doing things makes little sense.) This kind of implementation makes more sense in a pure OO world where the object that has been updated has a specific listener that listens for an update.

I think we'll be using a parameterised pull method like this:

Code: Select all

UnitIdle(unitID)

or

Code: Select all

struct UnitIdle {
  int unitID
}

that is then passed to handleEvent().
If the AI needs more information about the unit in question, then some callback mechanism will have to be used.

Here the advantage is gained when the AI chooses not to use the UnitDef information, since less data needs to be bridged. However, if the AI often needs information updates, we start to rely on the callback mechanism after every handleEvent(), and we ought to have used the Push way of doing things.

Mixing the two particularly makes sense for events like EnemyEnterLOSEvent, where we are usually interested not only in the event itself, but the coordinates of the enemy. Using a Pull method only is somewhat cumbersome, and yet not all the information (like UnitDef), needs to be pushed. So we work with a hybrid struct like this:

Code: Select all

struct EnterEnemyLOSEvent {
  int unitID;
  float3 position;
}

and if we need to know more about the unit, we can use the callback mechanism on the unitID.

Advantage:
We have the best of both worlds: frequently required data is pushed as usual, and other information can be retrieved with the callback mechanism (which we'll need in some form anyway).
Disadvantage:
We'll need to think about where we want to supply information in the push, and where we want the information to be gathered via callback.


Callback

Things brings me to an important point of this post: how do we want the callback mechanism to work?
Will we be using functions here, like:

Code: Select all

UnitDef* getUnitDef(int unitID);
float3 getPosition(int unitID);
...

or are we going to go for a callback that works like the handleEvent method:

Code: Select all

void* callback(int callbackID);

where we return whatever structure is expected.

Any thoughts?

[zenzike]

As a follow up, I figured one minor advantage of HandleEvent approach is that you can put exception handling in a single place (around the switch statement).

Actually, the advantage isn't so minor: in the same spirit as exception handling being in one place I can think of a couple other uses. So here are a couple advantages of the handleEvent() method:

Exceptions
All exceptions are handled in a single place -- this will mean we don't have to worry about adding exception handling every time we create a new event, since the handleEvent() takes care of this.

Logging
Logging information is in a single place which means debugging becomes much easier if we need it.

Profiling
We can also do things like profiling: working out how often a particular call is made, and how often we need to take that kind of information into account.

As with exception handling It would be a pain to change each of the functions to take into account small changes like these, whereas making the changes in a single handleEvent() function is much easier.