Interface Redesign

[hoijui]

uff!

a C style vtable is... a list of function pointers?

so for AI->engine we use either function pointers or public engine functions. If i get it right, the public function thing is code wise easier (to understand and error check) but could cause problems when compiling/linking. Using function pointers, we had the advantage, that an AI can not send messages as if it were an other team, right?

Engine -> AI
i would prefer the Engine->AI side to be simple functions plus the HandleMessage() function with a void* pointer then:

Code: Select all

DLL_EXPORT void Update(int teamId);
DLL_EXPORT void Init(int teamId);
DLL_EXPORT int HandleMessage(int teamId, int fromId, int messageId, int messageTopic, void* msg);
DLL_EXPORT int Save(int teamId, const char* fileName);
DLL_EXPORT int Load(int teamId, const char* fileName);

I would like to have a fileName for saving and loading, as streams are not portable (not to other languages then C/C++ )

for languages like Java, we can use a wrapper C interface, which will map the HandleMessage() calls to simple C functions.

AI -> Engine
for implementing it, and for making the code better readable, the public function idea seems better to me. how it is it has to be figured out only once per platform, and then we can use it... i think that would be ok. if not, i am ok with the function pointer way as well.

[zenzike]

I agree that the interface should be something simple like:

Code: Select all

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

I'm not sure if I'm right, but I think the handleEvent() function should expect the second argument to be one of the following structs, so that the (void*) can by typecasted:

Code: Select all

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

Here I'm indenting to show a class hierarchy.

I think it's better to call these Events rather than messages, since they are events that occur in game, whereas a MessageEvent would contain the contents of a message typed by a user.

Also, maybe I'm being fussy but IAIView seems a better name than IGlobalAI, since after all, we are implementing an MVC pattern, with the model being spring itself, the view is this class, and the callback is the controller (which ought to be called IAIController). At least then the pattern would easily be spotted by others.

[hoijui]

to call the files after prototypes of a pattern is bad. the pattern is only an abstract help, it is not the main thing. the names should describe the real thing, not mirror one of the abstract patterns in use behind the scene.

i would like all the current events to be transmitted in their own function, and not through HandleMessage(). that is how i ses it: the HandleMessage() function is only there to not break compatibility when a new event is added. in practise, all the events/messages will be mapped to functions anyway, if not by the interface, then later in the implementation. so it makes no sence to send them through HandleMessage() - now that we are creating the interface - as there is not yet anything whichs compatibility we can break. it would only mean more coding and therefore more chance for errors, more time involved, and more parsing and mapping to events and then from events to functions again. generally: unnessesarily generate complexity, as Tobi said before

we can even send the events we currently have as structs (e.g. ChangeTeamEvent) through functions, and not through HandleMessage().

i agree though, that we may need better names for things (i would also prefer Event over Message eg.).

am working on the next draft currently.

edit: typos

[AF]

All messages and events and communication should occur through the same method, HandleMessage(). It is important that we are consistent otherwise a week or two later some dev will come along and add another C function and another and we won't have 1 function we'll have 30 or so.

Highly anticipated fatal API design flaw of the future:

Does this API use a C function or HandleMessage()??

And what happens if we decide to change how a particular C function works? We cant overload so we're left with RandomCFunctionVersion2(), what happens when the AI tries to call a C function but the function no longer exists because it was deprecated in a newer spring and this is a version agnostic AI? During development we'd need an svn build for svn spring and an svn build for normal spring.

All messages go through HandleMessage() in a generic way. This keeps the API consistent, makes it less confusing, and futureproofs the entire system.

What's more it simplifies the AI side that rebuilds the C++ interface. Do you really want to have special case code for each and every C function? If you add C functions now, I guarantee you more will be added later if not by you but by other developers.

And remember, jelmer and I said exactly the same thing about the current Interface when we added the generic void* based methods, and it worked for a few weeks then people started adding C++ methods anyway and ignoring the whole thing defeating the point and making the interface more and more unwieldy.

[hoijui]

Hmm...
when we use HandleMessage for everything, and we hava EventX struct which changes from:

Code: Select all

struct EventX {
float arg1;
int arg2
}

to this:

Code: Select all

struct EventX {
int arg1;
int arg2
}

then we'll break compatibility the same way as if it would be a function, no? i can not see the difference. as the struct is used by engine and AI code, the ai that expects arg1 to be a float will fail to compile, and if we use the one compiled in the past it will fail at runtime...

[imbaczek]

you want a void* or a biiiiiiig union. in both cases, you can add stuff without breaking compatibility. (i gather that both break tools like swig, right?)

[hoijui]

i don't know about unions, but void* are ugly with SWIG, yes, but thats not really such a big point... i can still wrap everything to simple c functions after the events arrive.

but why would compatibility not break with a void*?
yes, the interface will not change, function wise, but the event structs are part of the interface as well, and they would change.
-> as soon as we cast the void* to the appropriate event struct... it will fail (either when casting or when we want to access arguments that have an other type or were removed). i mean.. if the argument of an event/function change, the code of the AI has to be changed. there is no other way.
the only pro of events is, that if we only add an argument, but not remove or change any, then it is still compilation compatible, but the AIs maybe miss important information, and the AI dev will not know it as his AI still compiles. in the other case, using functions, we would see which AIs are missing (possibly important) information the should be handling.

edit:
so is it only about this?
compilation compatibility when an argument gets added?

[zenzike]

I think that the point is that each of the structs like UnitCreatedEvent, UnitIdleEvent etc carry the appropriate interface in their structures -- their base types will not change often.

What does change is that new events will get created, for example, I forgot to add WeaponFiredEvent in my list of structs; in the old system, we would have had to add a new function (which breaks compatibility), but in the new system, we just add a new struct to the end of the list -- no compatibility is broken, and we stick to the original pattern so everybody wins.

Of course, there are ways around changing the interface of an existing event. For example, if we decide that, say, the EnemyEnterLOSEvent requires a time stamp argument that wasn't in the original struct, then we can create a new struct:EnemyEnterLOSTimedEvent that is a "sub-struct" of EnemyEnterLOSEvent. That way, compatibility with AIs that used the old EnemyEnterLOSEvent still remains, and yet the new functionality can optionally be implemented in the new event. The bonus is that by sub-structing, a new developer can clearly see that the two are linked, and that one is simply a refinement of the other. Again, everybody wins.

I might have got the wrong end of the stick here, so please correct me if I'm wrong.

[hoijui]

aehmm... no.
if an event (its struct) changes, compatibility IS broken!
except in your example, because you only ADD an argument. That is the only change where it works, and it has the downside that the AI devs will have to know that this change was made, in order to be able to profit from it (use it).
This small advantage, which may once ocurs, is just not enough to me, to justify the more work, more complexity and more wrapping the All-Events-Way brings, in comparison to the functions way.

an other reason why i think it iss not good:
Again, the C interface will not be used by AIs directly, as its uncomfortable. It will be wrapped into a C++ interface, Java interface and so on (all on the AI side). In my eyes, these interfaces should always be up to date, and suport the latest things.. whether the AIs do or not, is an other question. keeping AIs compatible should happen at this layer, not at the basic C AI interface layer. this can be done easyly by this interfaces, providing default empty implementations for new functions in the interface, or mapping the new arguments of an event/function to the old function and its argument set. the new function can still be used by overriding the new function (in C++ or in Java eg). the only downside of this is, that we have to change all the languages interfaces, wehn we add a new function (to the basic C interface), as otherwise they will not work anymore. but i think that is a good thing! they should alwyas be up to date. if one interface is not maintained anymore.. it should be moved out of the trunk in SVN, and if the maintainer comes back a few months later, or someone else wants to bring it up to date, it can be actualised and moved back. or the one that updates the C interface also updates all the other languages interfaces, which should be easy, as there are a lot of examples hotw to do it in the files he will have to edit. if a change is really needed, i think its ok to request this work from the one that wants the change. the whole sum of these changes may never as much time as it will take to generate all these events and their inheritance structure, creating them and parsing them back to fucntion calls. spring wont live for a hundred years.

[hoijui]

The draft with simple functions only, no HandleMessage(), using function pointers for the callback (AI -> Engine).

As i think it would be best now.

[AF]

Joiju, as I understand it adding parameters to a struct on the engine side at the end of the struct will not break compatibility with AIs. Structs iirc are ordered sequentially in the order they're deifned in the header and as long as the padding is correct there may be extra variables in a struct than are defined when it is passed to the AI, that it may not be aware of.

As long as the AI can access what it expects there to be in the struct and the size of the variables in the struct it expects to find have not changed, which it wont as they're all going to be primitives, then the whole point is irrelevant.

Your also failing to see the point that this isn't just about compatibility, its also about protocol design. Your failing to see the usability and the psychological aspects of this. Your trying to see it from an implementers point of view, and it will make it more frustrating for you when you come to do the AI side and you cant magic it away in a generic formula and you have to handwrite extra structures to piece together the special case functions you used at the C level and fold them back into the generic messaging at the C++ level.

You are making a HUGE mistake here by shifting towards individual C functions for things. You're sowing the seeds of gigantic headaches for us AI developers in the future, the very problems that we are trying to avoid by starting this in the first place.

I'm afraid if your unwilling to listen to me or to heed everyone else in this thread with this then Id have to withdraw my support for this endeavour and seek other solutions. We have a design, we have a plan of action, its been discussed, please don't deviate from it, and at least explain rationally the reasoning behind your decisions in a clear and coherent manner.

[hoijui]

BD, you shall not ask for rational arguments with THEATHRALIC words. you shall see. if you can not, give me your other ear to smash on.

The design of the protocol is a means, it is the way to the end, not the target.

That it is possible to add a value to an event and not break compatibility, was said before by zenzike and me in this thread. therefore, there was no need to explain that.
I made an argument, which renders this circumstance both positive and negative. you did not bring a contra argument against my negative argument, but instead repeated the positive argument. this was useless.

As long as the AI can access what it expects there to be in the struct and the size of the variables in the struct it expects to find have not changed, which it wont as they're all going to be primitives, then the whole point is irrelevant.

The same error in your logic, still (plus some more); explanation:
using events, compatibility is not broken on a change, if all of the following criteria are met:
* An argument is added
* It is added at the end of the struct
* If this argument is ignored, the event can still be used by the AI, the same way as before the addition of the argument
compatibility is broken, if one of the following criteria is met:
* An argument is removed
* An arguments type changes
* The order of the arguments changes
* An arguments meaning changes, but the type remains the same
* A new event is added, which will render an AIs state inconsistent, if it is not handled

I do not know what you mean with special case functions.

About folding back to generic messages at the C++ level:
For perfect reproduction of the current C++ interface after the C interface, on the AI side:
You are right. For that, i would have to write special code that translates from a function call to an event call. This is not the way i want to do it. I explained my way, which you must have over read, so i explain it again:
in my model, there are no Events. also the C++ interface would not use events. there would be a default empty implementation of a virtual function (for each new event), which assures the same degree of compatibility we would have with an event. Both techniques make the AI ignore the new event, as long as the code is not adjusted to use the event, but not break compilation compatibility.

From an AI devs point of view:
* If i do not change my code, the event is ignored, which possibly renders my AIs state inconsistent, whether using functions or events.
* To know that there is a new Event, i have to:
- with events: look into the engines header file where the event structs are defined, and check if there is a new one.
- with functions: use my C++ IDE, and let it show me the list of local functions, and check if there is a new one.
* To use the new event:
- with events: parse the new event. make a new function actually handling the event. call that function
- with functions: override the function

I am not sure about this:
KAIK crashed when a unit was captured, as it did not handle ChangeTeamEvent.
someone else may call it a FATAL fail for events.
I would say, this would possibly happen less with functions, as a new function in the concrete interface an AI dev uses is easier found then a new event struct in the header of the engines C AI interface (which no Java, C# or C++ AI dev should ever have to look into).

I got convinced by Tobi through rational arguments that functions are good, namely:
* no parsing to and from something -> fewer code, less coding work, faster code execution, lower complexity -> fewer errors overall
* highest possible compatibility
before that, i got convinced not to use VarArgs, by other rational arguments. i explained why i now think, that functions would be best. it therefore seems very possible to convince me, using rational arguments, that events are better.
but...

You're sowing the seeds of gigantic headaches for us AI developers in the future, the very problems that we are trying to avoid by starting this in the first place.

I'm afraid if your unwilling to listen to me or to heed everyone else in this thread with this then Id have to withdraw my support for this endeavour and seek other solutions.

is not quite something that could do the job.
not to mention that it is incoherent with this:

at least explain rationally the reasoning behind your decisions in a clear and coherent manner.

We have a design, we have a plan of action, its been discussed, please don't deviate from it.

Saying "We have a design", is obviously incorrect. It would have to be: Each one of us has a concept. Unless you are using the word "We" for your majesty only, of course.
This thread is for designing the interface. for creating a commonly agreed upon concept, influenced by arguments of all of us, to make it as good as possible. it is for letting everyone know how it will be, and why it should be like this; everyone that is affected or at least interested.
It is not for sticking to the once made, initial concept. The discussion in this thread is not yet over, simply cause we do not yet agree. if you referred to an other discussion, that took place out of this forum, then it was a mistake of you to open this thread, and that you did not explain this concept in enough detail.

I am not an instrument, someone that codes something the way you want it. i am a coder - more or less active - in an open-source project. i am learning and trying to let my skills play a part in the design process.

An Example
function way:
Someone changes the C interface, adding an event, and forgets to adjust the C++ interface.
-> compilation fails
Everyone knows that something is wrong, we look at the SVN submits and find out who it was, we can explain it to him and fix it. the new function/event is visible in all the AI interfaces.
events way:
Someone changes the C interface, adding an event.
compilation is not broken, but the new event will stay hidden till it breaks the logic of AIs, as the AI devs will hardly find out about the new event otherwise. (smells a bit like psychology to me)

i may have a bit an other idea of the target of the C interface, then you have. but is yours the correct one? mine the wrong one? or is yours at least better? is yours the better one cause it was first? because without it, i would not have my own idea? is it better cause you made your own AI yet? or is it just your right to have the better idea, cause you are here longer then me, and wrote much more stuff for spring? that much, that you do not even have to explain why your idea is better? is your idea better cause you have been using C more then me? because you have more experience in development then I do in general? if so, how do you know that it is so? do you get it?
you are too arrogant DCP. step back, and see.

sorry for the repetition. there was some in this post and others.
Sorry also for the again bad structure of the post.
But maybe you are missing basic inter human communication skills and philosophical ones, which make your posts hard to read for me.

respect? please?

Joiju

[AF]

One of the major reasons people discussed as a major disadvantage to spring AIs was that every release we had to recompile and re-release all the AIs due to C++ ABI compatibility issues. People discussed the idea of a new itnerface as a way fo escaping that, which then devolved into people complaining that the current interface was cumbersome and ahrd to udnerstand and ill designed.

The idea of reintroducing that at a base level deliberatley by design is not very enticing. Everywhere I have been, all the communities, all the books, all the articles, all the code bases I've looked at have all stressed the need for highly modular flexible generic code for AIs.

One note that should also be observed is that the design of the IGlobalAI* class heavily influenced the designs of the AIs built on top of it. By providing a foundation class that is a simple and primitive and generic as possible we allow the greatest flexibility and design choice for the end developer who arrives on the scene.

I say we use structs. If you want the whole type safety and the comfort of IDE help such as intellisense you can use object factories and wrappers on the AI side of the divide.

Your arguement of adding events doesn't quite hold sway either. An AI will break regardless of whether that event is implemented usign your method or my method.

However, neither are needed as there is an alternative solution. We can allow the AI to send the interface version it expects to the engine, and the engine can then reject the AI if its version number is incompatible for whatever reason.

However any event such as you proposed could break AIs like that is likely to come from lua gadgets, for which a generic messaging system across the C boundary will be needed anyway unless your willing to enforce strict message formats on lua coders.

While you are a coder and you are willing tow ork on this, do bare in mind that we AI developers who are already here have extensive code bases and we thus have a vested interest in the AI interface we use and its design.

[hoijui]

..Thanks very much for explaining some

You said that a lot of people here think the current design is bad, and it should be more modular, and lots of books and other communities and so on say the same. Though, i still don't know why, which is actually what i am interested in, what convinces me.

That the AI will break when adding an event in your and my way, was what i said as well. My argument was, that with my way, it would be easier to see for the AI Dev that there is a new event.

However, neither are needed as there is an alternative solution. We can allow the AI to send the interface version it expects to the engine, and the engine can then reject the AI if its version number is incompatible for whatever reason.

Seems like a good idea. Though it works equally well, wether using events or functions.

When using events, it is easy to make a function based basic C wrapper on top of the event based one. The other way around, using functions and then create an interface that converts them into events is definatly more ugly. Something you said made me see that.
Is this the main reason? i would understand it then.

Should JAI be based directly on the Event system as well, and then build up an OOP interface uppon that, all in pure Java already?
if so.. then i would have to insist that we not use a void*, but a base struct, and no nested structs. I just made a test -> struct inheritance is supported by SWIG. As i got it, this was the idea anyway (at least yours and zenzikes).

Two questions then:
* Is it possible that i work on the C interface, ignoring the LUA stuf, and adding that later? or do we need to be sure about the concept for LUa before we should start coding the C interface?
* nameing conventions for the events:
should the names be based on the C++ interface structure?
examples:

Code: Select all

#define EVENT_AIINTERFACE_GET_UNITDEF_SPEED "AIInterface_Get_UnitDef_Speed"
#define EVENT_AIINTERFACE_GET_UNITDEF_SPEED "AIInterface:Get_UnitDef.Speed"

will we separate events into groups? for example:
* Engine -> AI
* AI -> Engine
* Engine <-> AI
* AI <-> AI
or will we just keep them in one single list and naming convention?

[zenzike]

Hoijui, it seems that your objections to the handleEvent() function are these:

compatibility is broken, if one of the following criteria is met:
* An argument is removed
* An arguments type changes
* The order of the arguments changes
* An arguments meaning changes, but the type remains the same
* A new event is added, which will render an AIs state inconsistent, if it is not handled

I agree that these are problems, but I don't think that they are likely to happen in reality. First, if an argument is removed, type changed, reordered, or has a changed semantic, then this will be because some inherent primitive Event is being changed. If that is the case, then there has probably been a big shift in design decisions, and we'll have to change the way the AIs work anyway (whether we're using functions or Events). Unfortunately there is no way around these kinds of changes other than re-coding the AIs, since compatibility has fundamentally been broken by the engine. As AF mentioned, this can be caught by passing an interface version number from the AI --> Engine that ensures that the two are in sync.

Now if a new event is added, then the AI will simply be incompetent: not inconsistent. Adding an event that is not being handled means that the AI does not take into account some information that it should (and will discard it instead). If the AI wants to take into account the new information, it will always need more code, whether we use functions or Events, so again, there is very little difference here.

Your final example about changes in C interface leading to failed compilation is the most compelling; it would be nice to catch problems at compile time between the C-->C++ interfaces if somebody forgets to add a change to the C++ interface. I'm not convinced that compilation will fail though since I see no reason why it would (could you explain that?), and I think that this is the same whether we use function calls or Events.

There are lots of advantages to using Event passing in a handleEvent() method: AF has mentioned a lot in the previous post to this one, and Tobi made a good argument about there being fewer casts to worry about. There's also the added advantage that it's conceptually simpler to handle all events whether new or old in the same way.

[zenzike]

Should JAI be based directly on the Event system as well, and then build up an OOP interface uppon that, all in pure Java already?
if so.. then i would have to insist that we not use a void*, but a base struct, and no nested structs. I just made a test -> struct inheritance is supported by SWIG. As i got it, this was the idea anyway (at least yours and zenzikes).

I think we should use something like this:

Code: Select all

handleEvent(int eventID, void* event)

with logically nested structs (like I described before). The idea is that the event is passed through a single function called handleEvent from the engine to the AI interface. The interface then does something like this:

Code: Select all

handleEvent(int eventID, void* event) {
switch(eventID) {
    case UNIT_CREATED:
        UnitCreatedEvent e = (UnitCreatedEvent) event;
        // handle the e event appropriately.
        break;
    ...
}

the point is that once we have received the message, it gets bound to the appropriate struct type immediately, and from this point on, the compiler can type-check the event e in the appropriate way.

We need a void* because we are just passing a data-structure that has no real base type -- C won't understand inheritance.

I think it would make more sense for JAI to build on the C++ interface, rather than doing exactly this work itself, but of course, you have more experience in this field.

[hoijui]

ahh sh..
struct inheritance is only supported by C++, hm? :/
so we have to use a void*

the thing is, that i have to extend every event struct with a method like this:

Code: Select all

%extend AIHCAddMapLine {
   void* getVoidPointer() {
       return (void*)self;
   }
};

so i can use it with SWIG and Java. (this is done in the SWIG interface generation file, not in the code of the C interface itself)
it is no hard job, just annoying.

will see in the end, whether i will use the C++ interface or move to the C one.

Now if a new event is added, then the AI will simply be incompetent: not inconsistent.

KAIK crashing becuase it did not know about the ChangeTeamEvent proves that this can make the AI inconsistent, and crash the game. We had to specify manually when adding an event, wether it can break compatibility or not. The ChangeTeamevent can for example, others maybe not (lets say: LineDrawnEvent).

Your final example about changes in C interface leading to failed compilation is the most compelling; it would be nice to catch problems at compile time between the C-->C++ interfaces if somebody forgets to add a change to the C++ interface. I'm not convinced that compilation will fail though since I see no reason why it would (could you explain that?), and I think that this is the same whether we use function calls or Events.

When we add a new event (in my way a function), we will add it to the Engine sides C interface implementation and to the AI side interface aswell (which is only a header file). the C++ interface imports the AI side C interface header file, which now contains a new function, which the C++ interface implementation (the AI side) does not yet implement -> compilation fails.
the same would happen with JAI eg, if it would use the C interface directly -> compilation would fail.

There are lots of advantages to using Event passing in a handleEvent() method: AF has mentioned a lot in the previous post to this one, and Tobi made a good argument about there being fewer casts to worry about. There's also the added advantage that it's conceptually simpler to handle all events whether new or old in the same way.

I did not find a single argument pro events in AFs last post. When using functions there are less casts then when using events. the only thing that is simpler with events, is that the C interface will contain less functions, but it will contain more code (the structs), and be more complex (though still relatively simple of course). the implementation wil also be more complex.

[Kloot]

I am not sure about this:
KAIK crashed when a unit was captured, as it did not handle ChangeTeamEvent.
someone else may call it a FATAL fail for events.

Incorrect. Please do not use an example that you
don't know the specifics of to push your own ideas.

struct inheritance is only supported by C++, hm? :/

Yes (as are struct member functions).

[AF]

Could you not wrap the C function with void* in functions using the structs and use those for SWIG instead?

The point of modular oo code in AIs is to increase flexibility and code re-use, things like replacing algorithms easily and swapping out components in the code or at runtime. It also greatly simplifies various pieces of code and allows for greater options. Different components can then be reused in various contexts.

With events, to add more you would need to modify the engine, whereas here you need only add a struct to your AI. It also removes the distinction between Lua game events and engine hard coded events, as well as introducing a level of forward compatibility. I could release my AI using svn code and disable components if an earlier spring version is detected and release a single binary, possibly taking advantage of SDK improvements and fixes.

[hoijui]

ahh, modularity inside the AI...

the last paragraph gave me some more ideas of what is good on it.

Could you not wrap the C function with void* in functions using the structs and use those for SWIG instead?

this way i would need a function for every event, wouldn't i? the whole joke of it is gone then, as it means adding a new function for a new event. or did i get it wrong?