Interface Redesign

[hoijui]

about the callback:
i think it is pretty clear that we use either individual functions or HandleEvent() in both ways. not individual functions for Engine -> AI and HandleEvent() for AI -> Engine or vice versa.

the other thing:
it seems to me, that what you want to discuss here, has already been discussed when the current C++ interface was designed.

UnitDefs are meant not to change during a game. if they will change (which, as i got it, is possible with Mods using LUA), we can possibly use something like a UnitDefChangeEvent (in the future). In general, an AI will load all UnitDefs in the beginning, analyze them, and store them in lists. later on, it only fetches unitDefIDs associated with units, which it then uses to refer to the unitDef data stored in its own lists. Also, when a unit gets idle eg, the AI will already know which unitDefID is associated with this unit, as it stores this in an internal list, so there is no need to send the unitDefId in the UnitIdleEvent, and we will never send the whole UnitDef struct through the C interface, but only single values of it.
edit:
...woops
no we would not have to pass single values :D
... why was i just thinking so? :/
we could have a GetUnitDefEvent which contains all values (ids for complex values, that are made up of structs or classes).
sorry! :D

[Tobi]

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.

hoijui showed it as demonstration after preprocessing

But you are right yeah, using magic numbers in the actual code doesn't make sense.

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.

Good point, I hadn't even realized we had this left open for discussion.

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).

I've never seen such implementation issue.

Another disadvantage I can think of is that it's often pretty unclear which data should be pushed, and I fear you will end up with a random subset of the usable data being pushed. In other words, you will still need a callback interface to get the missing information.

It would seem like a waste to me to push for example all unit stats that are available when e.g. experience is never used in any AI in EnemyLeftLos.

And when for example only unit position is pushed, this introduces an inconsistency in getting unit parameters: unit position would be available in event arguments, but unit health would need to be queried. And as a "bonus" you can choose to query unit position too, and hope you get same value back as what's in the event

Currently my preference goes to supplying only the information that's really specific to the event, ie. the unit ID in case of EnemyLeftLosEvent, the frame number in case of Update, and not much more.

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?

For callback my preference goes to separate functions, but called through a function pointer.

A generic callback function seems like a useless complication to me for the callback interface, not in the least because of the trouble of returning a pointer to an object:

• it can't just be created on the stack in the engine because then pointer would be dangling on return of the callback function.
• It can't be allocated on the heap without a bunch of code to automatically garbage collect it a later frame.
• It can't be allocated on the heap and freed in the AI because they may use a different C runtime (ie. different heaps).
• It can only be made a global variable engine side (or member of the object representing the AI, but that's still pretty much global), with accompanying issues of people still trying to free the pointer, callbacks being non-reentrant, etc.

Of course one could make it like this:

Code: Select all

void callback(int id, void* data);

Where the AI allocates space for the data and the engine actually fills it, but:

• I do not think this is very intuitive (because of the "out" parameter).
• It wouldn't have as good compatibility because the callback data structure couldn't be made bigger in the engine: it would write out of bounds of the area allocated by older AIs.

Thats why my preference goes to separate functions, which return either a primitive types, or pointers to structures that have a lifetime that far exceeds the single callback call (ie. UnitDef).

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:

Good points, I'm convinced now handleEvent is the way to go for the engine->AI communication.

[hoijui]

hmm.. k :D
all these arguments.. i had no idea about that :D

Engine -> AI: HandleEvent() with Event structs
AI -> Engine: functions with primitive arguments (or Event structs?)

i would be happy with that too. Engine -> AI are much less Events anyway (about 20, compared to around 100 - 200 the other way around), and can be redirected to a function per event in little time and code.

[zenzike]

It's pretty clear that we're all quibbling over the details of how to implement the Observer Pattern [1,2], where the spring engine is the Subject with various AIs that are the Observers.

Let's look at the different parts in more detail, and see if it sheds light on what we need to be thinking about.

Observer: AI
update()
The update() function is called when the Observer needs to know about a change in the Subject.

We've already discussed the nature of the update() aspect of the Observer; I think we now all agree to use the handleEvent() way of doing things. The reason it made sense for the update() to be a single function (handleEvent()), is that we have a single point of notification -- the Observer only needs to observe things in one place, which keeps code tidy, and the Subject needs to update its observers in only one way. We've already discussed many of the other advantages (other than this conceptual bonus).

Subject: Engine
notify()
The notify() function is called when the Subject state has changed and the Observers need to be notified.

As I understand it, the notification occurs on a frame-by-frame basis and results in the UpdateEvent. It also occurs as a side-effect of in game engine simulation, for example, when a unit is damaged, created, enters LOS etc.

getState()
The getState() function returns the state of the Subject.

In our case, things are a little trickier, since we're not actually interested in the state of a single object, since the engine is made up of many other objects, and those are the states we're interested in.

That's why we have different getState() functions, and why I totally agree with Tobi:

Thats why my preference goes to separate functions, which return either a primitive types, or pointers to structures that have a lifetime that far exceeds the single callback call (ie. UnitDef).

But the decision we have left here is: what level of granularity can we expect for structures to be returned in the callback?

I'm of the opinion that we should choose the finest level of granularity that is reasonable, whilst loosely coupling the AI with the engine's implementation details.

Effectively, we are trying to gain information about several different object states, like Unit and Map. For example, I'm guessing we are interested in some sort of object like:

Code: Select all

class Unit {
  float health;
  float maxHealth;
  float speed;
  ...
}

One approach is to ask for this object state directly.
Advantage:
* The AI can request the state of a Unit and gets exactly what it wants.
We can use inheritance when units get more complicated: old AIs can still interface with the old Unit interface, and new engines can interface with MegaUnit which is derived from Unit.
Disadvantage:
* Too much data is passed to the AI, when all it wants is a specific part of the Unit state.

A different approach would be to go for something like this:

Code: Select all

float getUnitHealth(int unitID);
float getUnitMaxHealth(int unitID);
float getUnitSpeed(int unitID);
...

Advantage:
* Only the data the AI is actually interested in is returned.
Disadvantage:
* We need to be careful that we haven't strongly coupled the AI with the underlying unit state eg, what happens if we decide to change the format of the float3 type?
* More code maintainence for the engine developers as the number of functions increase or the underlying storage types for Units change.

In either case I think it makes sense to wrap the callback in class like this:

Code: Select all

class Callback {
  UnitDef* getUnitDef(int unitID);
  Map* getMap();
  ...
}

so that we need only look to one place to see what the engine is exposing.

setState()
The setState() function is the means by which the Observer can change the state of the Subject.

So far we haven't drawn the distinction between the getState() functionality and the setState() functionality of the callback. I think that there's an important distinction here because the way we interact with setState() is conceptually very different.

One way of implementing setState() is by having multiple functions:

Code: Select all

void setUnitAttackTarget(int unitID);
void setUnitMovePosition(float3 pos);
...

This would be a way of mirroring the getState() way of doing things.
Advantage:
* sending events to the engine is easy to understand.
Disadvantage:
* If a new version of the AI tries to play on an old engine that doesn't support a function, then things break.

I think a better way though, would be to use a sendCommand() method like this:

Code: Select all

int sendCommand(int commandID, Command c);

Where the return value is the success of the sent command and commands are things like:

Code: Select all

Command
  UnitCommand
    UnitMoveCommand
    UnitAttackCommand
    ...
  DrawCommand
    DrawLineCommand
    DrawCommentCommand
...

Advantage:
* We get all the advantages of the handleEvent() arguments, but on the engine side. In particular, we have one point of interface to debug when things go wrong, making exception handling much easier.
* The engine can deal with Commands however it wishes -- if an unknown command is sent (because we're using a new AI version on an old spring engine), the engine can choose to ignore the command.
* Since we'll be using handleEvent(), it nicely mirrors the way the two parts interface.
* We're making use of the well known Command pattern [1,3].
* I hope this is how the user interface works already!
Disadvantage:
* Maybe more complex to understand at first?

In my opinion, the sendCommand way of doing things makes more sense, since it's more future-proof, and actually opens us up to other interesting ways of interacting with the engine: for example, we could start having text-based control, rather than mouse based (imagine ASCII spring!)

edit: I forgot to include the addObserver() and removeObserver() functions; this is obviously the addAI(), and I'm not sure we have a way of removing AIs, though I don't think this matters.
edit: typos.

[1] Design Patterns: Elements of Reusable Object-Oriented Software by Gamma, Helm, Johnson and Vlissides
[2] http://en.wikipedia.org/wiki/Observer_pattern
[3] http://en.wikipedia.org/wiki/Command_pattern

[hoijui]

wow.. :D

the callback can not be a class, as it is a C interface. it could be something similar though, as in my draft; a struct with function pointers.

spring is using commands. the AI interface is quite close to the internal functioning of spring. i would say that we should stick to that, if possible, and not create an other meta model of everything in spring accessible to AIs. this makes our work much simpler, and would not generate extra complexity. also, for the C++ interface, we would have to convert from the spring C++ model to the C AI meta model and then back to the spring C++ meta model again.

a good question is the one about granularity!
i would say, that we use the exact same level of granularity as used in the C++ interface. this means for example:
* UnitDef is transferred as a whole
* Unit specific stuff that changes (isClocked, isRadarVisible, pos, ...) in separate functions
* UnitResourceInfo in one struct
it seems to be ok for me, and it would again ease the job of creating the C interface and the wrapped C++ interface.

[AF]

One of the pet projects of the past of mine involved planning and designing a new interface for spring the the intention it could be used in other game engines. We should not shoot ourselves in the foot if possible, and we should not introduce possible dependency on engine strutures that could change at any time in the svn.

I also stress that consistency is extremely important. We should nto approach API design from a purely technical standpoint and should factor in the usage and interpretation of the API aswell as keeping it consistent enough that future additions take the intended form and that the interface can be extensively futureproofed.

[Tobi]

zenzike: very good post, nice way to get it conceptually a bit more structured

Command pattern seems way to go in particular because that's already used in rest of spring code for sim commands. Also this way it's very easy to explain that e.g. Commands may only be giving during execution of an update event (in case we have such constraints), while the state can be retrieved anytime.

Plus as you say engine could then ignore commands it doesn't know about and return failure.

For getting state (getState()) this extra compatibility doesn't really make sense anyway I think, because there's not really a good way to return errors to the AI I think. I mean, no one checks return values, right? AIs would just crash if they got NULL pointer back because engine didn't support particular state query.

To summarize until now I think we have:

Single entry point handleEvent() in the AI which is called by the engine if the AI needs to be notified about something. Forward and backward ABI compat because if engine doesn't notify AI about event nothing bad happens and if engine notifies AI about event it doesn't know about nothing bad happens either.

Set of callback functions which return parts of engine state. (Data retrieval for AI.) Only forward ABI compat (older AI runs on newer engine), but both forward and backward API compat. (IOW AI author has to program against lowest engine version he wants to support)

Single entry point for giving engine a Command. (Command pattern)
Forward and backward ABI compat because engine can ignore commands it doesn't know about and AI does never need to send a command it doesn't yet know about.

Then only things left are some implementation details, for handleEvent these are clear I think, but the other 2 points haven't been discussed that much yet.

[hoijui]

so you want a single function HandleCommand() which is used for giving commands to uints and also for drawing lines eg? everything that changes the game state somehow?

currently, there are the GiveOrder() and GiveGroupOrder() methods (for giving commands to units an gorups), and the HandleMessage() method, for generic commands that were added later (eg used for drawing on the battlefield). i cant think of any other callback methods that would change game state. should not be too hard to map these three to a single function.

[zenzike]

so you want a single function HandleCommand()

Yes, although I think we probably ought to call it createCommand(), since it's actually a request for the engine to create a command and (possibly later) execute it. The idea here is very similar to the handleEvent(). The advantages are those we discussed previously about handleEvent(). The createCommand() has this declaration:

Code: Select all

int createCommand(int commandID, void* command);

and returns 0 if successful, something else otherwise.

Here's an example of how createCommand() might work for a BuildUnitCommand, with the following structure:

Code: Select all

struct BuildUnitCommand {
  int builderID;
  int unitDefID;  
  float3 position;
}

A new structure is then created an populated with data by the AI. Let's say we used the variable command, and that the constant int for a BuildUnitCommand is BUILD_UNIT_COMMAND. The AI then calls the createCommand() function like this:

Code: Select all

createCommand(BUILD_UNIT_COMMAND, command);

The engine then receives this function, and can use the commandID to cast the command into an appropriate structure. The relevant data is then read out of the structure, and the appropriate action is taken (presumably the action is to create an in-engine command, that will be similar to what the AI has sent. It's important to have this layer of abstraction here, since the engine might want to populate its internal version of the command with more information, like who sent the command, when etc, but the AI doesn't need to know about this).

i cant think of any other callback methods that would change game state.

Nor can I, but the great thing about using this mechanism is that we can always add more later, and nothing breaks.

edit: typos.

[zenzike]

We should not shoot ourselves in the foot if possible, and we should not introduce possible dependency on engine strutures that could change at any time in the svn.

I agree entirely with your post. I think for the most part my previous post manages to separate the AI and the Engine pretty generically.

There is still a problem with the getState(). To be honest, the most immediately practical thing I can think of is using a whole bunch of functions as before, like:

Code: Select all

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

This is far from ideal, and suffers from the problem that if a future version of an AI requests data from an engine that does not provide it, then we crash. Do you have any suggestions for something better?

A minor problem is that if the engine changes in the future, then we'll always need to support legacy datastructures, again this isn't ideal although no matter what the solution, this problem will crop up; and we would resort to an Adapter [1] to sort things out.

The closest I've come to a good solution would be to think about all the information that an AI would actually need in the most generic sense, and work according to the generic interface that will be required.

It's easy to do this with things like getUnitPosition(), and getUnitHealth(), but the distinction between an abstract data value and one that's just a side-effect of the Spring engine is more blurry with things like UnitDef. If we shift to this sort of generic interface, we'll be using an Adapter on the engine side to interface with the AI appropriately. This makes the engine free to change whenever it wants, and the AI will still work. However, we're still stuck with a fundamental problem: what is the basic datatype that these primitive calls would take?

Again, if you can think of something better, please let us know!

[1] http://en.wikipedia.org/wiki/Adapter_pattern

[Tobi]

The problem if you want to make the getState functions generic is that you'll be designing against lowest common denominator.

IOW, if there's even the slighest chance another engine you want to use the AI for doesn't have a unit property, you can not expose it to AI because then it might use it, making it incompatible with other engine. Same story when "other engine" is spring engine sometime in the future: if property has slighest chance to ever be removed it's shouldn't be exposed over such an interface.

However, this is contrary to what everyone wants, and I also think it's over-design/over-generalization.

So, I think the quicker solution to make functions like float GetUnitHealth(int unitid) etc. is the way to go.

Structures like UnitDef could be separated out in it's members entirely (lot's of get functions), or in principle one could hide entire concept of UnitDef from AIs. Not sure that's actually a good thing to do though.

Advantage would be that data can be arbitrarily moved between CUnit and UnitDef (for example when a value becomes lua-able per unit), without affecting AI interface.

Disadvantage is that AIs devs would probably go guess which values are mostly static, which may introduce more silent compat probs (which may be pretty much impossible to prevent anyway), like it having cached a value that in a later version fluctuates very much for some unit in some mod.

[hoijui]

@zenzike
what you showed with your example BuildUnitCommand... i like it, from a practical point of view.
the way unit commands currently work is way less obvious. one has to know which arguments a command excepts and add them in the correct order.

@Tobi
As you said, i think it would complicate AI development a lot, if it is not clear anymore which values are type specific or unit specific and which are fixed/static or rather change a lot.

As i see it, there are different type of values:
first, we have type specific ones which are generally static. then there are unit specific ones, which usually are very dynamic (current speed, facing, health, ...). If a type specific value is changed with LUA, it may still be relatively static -> change only 2 or 3 times during a game (in most cases?). so maybe we should make clear to AIs, which values are static. If a static value is changed, we would send a StaticValueChanged (eg UnitDefMaxSpeedChanged) event to the AI, so it can reinitialize some data (eg build a preference list). Unit specific values that may get changed by LUA, do not seem to be a problem, as they are dynamic without LUA already, and therefore need no special treatment.
(Yeah.. sorry, i am not really an AI dev, but i read through some code at least).

edit:
maybe AF could help here

[AF]

While it would be nice and easy to add stuff liek float GetUnitHealth(int unit), this is making a huge number of assumptions based upon the context this is called from.

I would also like to point out that AI developers should not directly use the C API for the AI development. They should instead use the C++ OO wrapper interfaces that are built ontop.

So instead of:

Code: Select all

float health = GetUnitHealth(unit);

we would have

Code: Select all

float health = unit->GetHealth();

Of which CUnit::GetHealth() will call the appropriate C functions.

I should point out that the value returned by this method in your example would always return the same value, whereas lua mechanics may suggest otherwise. Who and which AI is viewing the unit and who that unit belongs to can vary wildly and this may greatly affect the perception of that unit. For example if the unit is a flea with a maximum health of say 500, yet this unit has 1200 health, and the mod has 'mimic' units, then something is wrong and the AI is now unintentionally cheating.

I'm also not in favour of CreateCommand(). It is misleading and suggests that it Creates and returns a variable of some sort, something I would expect from an object factory. Instead PublishCommand() seems a more appropriate name.

[AF]

I'm also unsure of how exactly to define lua communication at a C level in a generic level, but the main problem there arises from a lack of understanding as to how exactly the call propagates from AI to lua gadget and how one would use it. AI developers are not lua experts, nor do they have practice writing lua gadgets..

[hoijui]

AF...
you think the functions are not good, but neither have an other idea?

what about the static/non-static difference? (not just AF)

did not understand your example. :/
how could it happen that the AI thinks the unit has 1200 health, while it actually has only 500? Is that, when the health is set to 500 by LUA? is it not possible to always report the real health to the AI?

[AF]

I said I do not believe CreateCommand is an appropriate name because it implies it does something it does not. Instead it should be called PublishCommand or HandleCommand.

CreateCommand implies that your passing parameters and getting a command in return, object factory behaviour, which is not what is happening.

Are you not aware of the RPG tradition of the monster called 'mimic'?

Lets say we have a brand new unit for the core called the 'mimic'. The mimic can put on a disguise so it looks like another unit. "Hey lets blow up the flea over there" ( but its not a flea at all its a mimix! ).

Or if we use an actual existing unit, lets take the fusion reactor and the decoy fusion reactor. Lets say Fusion reactors have 500 health and decoys have 1000. The AI can immediately tell which is which because any fusion with 1000 health is the decoy. Now lets say there's a lua gadget that makes it so the decoy 'appears' to have less health than it actually has depending on whose looking at it.

On top of that you need to pass in team and ally variables because if you don't you cant perform LOS checks.

GetUnitHealth(int unit)
GetUnitHealth(int unit, int observingTeam)

[hoijui]

sorry, i forgot to comment on your suggestion about the CreateCommand(), PublishCommand() thing. I agree 100%, and i would like HandleCommand() a little more then PublishCommand().

thanks for the explanation with the mimic! now it is easy to understand for me :D
now that i got it.. yeah.. makes sence to call that mimic

we will always know which is the observing team, on all callback method (eg GetUnitHealth()). in my draft it is simply done by always supplying int myTeamId as first parameter. i do not know of any other way... as i understood it, it could only be done with C++ function pointers (the engine giving pointers to functions of a class that has a member variable which contains the teamId), but not in C.
no matter how, we will always know the observing team.

Code: Select all

GetUnitDefSpeed(int myTeamId, int unitDefId);
GetUnitHealth(int myTeamId, int unitId);
IsUnitClocked(int myTeamId, int unitId);
<whatever>(int myTeamId, ...);

this problem seems to lie in the engine sides C++ implementation (of AICallback::GetUnitHealth(int unitId) eg), not in the interface, right?
does not mean that it is not important.. i just want to understand it.

[AF]

That suggests AICallback is not shared between AIs but that each team is given its own AICallback object. Im just speculating though.

[hoijui]

ahhh.. yeah true. i always though that this is the case. do not know if it really is. i guess it is, because when you call GetUnitPos() for an enemy unit out of LOS and Radar, it fails.
in CGlobalAI:

Code: Select all

	callback = SAFE_NEW CGlobalAICallback(this);

	if (!postLoad || (postLoad && !loadSupported)) {
		try {
			ai->InitAI(callback, team);
		} HANDLE_EXCEPTION;
	}

in CGlobalAICallback:

Code: Select all

CGlobalAICallback::CGlobalAICallback(CGlobalAI* ai)
: ai(ai),
	cheats(0), 
	scb(ai->team, grouphandlers[ai->team]/*ai->gh*/)
{
}

so yes, each AI/team has its own callback.

[hoijui]

made an other draft, which should reflect about the current idea. as nobody objected to my static dynamic stuff yet, i used that aswell.

it still includes nothign for LUA.

what do you think?