AI interface version is not used at all
Moderators: hoijui, Moderators
AI interface version is not used at all
I wonder why AI interface has any version while it is not used at all to inform about unsupported AI or transparent rolling back to old interface instead of a crash?
Currently AI compiled for 0.82.5.x will crash the game.
Currently AI compiled for 0.82.5.x will crash the game.
Last edited by slogic on 18 Oct 2010, 10:31, edited 1 time in total.
Re: AI interface version is not used at all
As I remember it, Hoijui didn't get around to implementing AI interface error checking, and there wasn't much need for it, and us AI developers never complained about it, so he got on with other more pressing matters such as the pureint branch.
Although a more graceful crash message or even abandoning loading the AI, continuing, and just spitting out an error saying loading failed, or a stacktrace at the very least would be welcomed.
Although a more graceful crash message or even abandoning loading the AI, continuing, and just spitting out an error saying loading failed, or a stacktrace at the very least would be welcomed.
Re: AI interface version is not used at all
Each AI already exports getLevelOfSupportFor function. So Hoijui just needs to update ENGINE_VERSION_NUMBER in aidefines.h each time AI interface has changed. That is all.
Of course, better solution is trying to load old interface module, which can be left in the interface folder after installing Spring over old one. But it needs much more time which nobody has.
Of course, better solution is trying to load old interface module, which can be left in the interface folder after installing Spring over old one. But it needs much more time which nobody has.
Re: AI interface version is not used at all
Supporting old interfaces would be nice, but I think that would lead to allsorts of maintenance issues and it would force all sorts of design decisions on us that we would regret later.
Instead, I'd be happy if we just had clear warnings and messages, and a graceful fallback, e.g.
This message is more precise, tells you which AI failed to load, why it failed to load, and what needs doing to fix it. This could be followed by the usual abandoning the attempt to load the AI and continuing as if nothing happened.
Instead, I'd be happy if we just had clear warnings and messages, and a graceful fallback, e.g.
This would be a massive improvement, as this is even better and clearer than what we had before Hoijui arrived here and rearchitected the AI interface, and even back then this was an issue of contention amongst those unfamiliar with how things worked.AI 1 (Shard v0.30) failed to load, interface version mismatch, interface version: 12, AI supports: 11. Please rebuild with the latest engine source
This message is more precise, tells you which AI failed to load, why it failed to load, and what needs doing to fix it. This could be followed by the usual abandoning the attempt to load the AI and continuing as if nothing happened.
Re: AI interface version is not used at all
indeed, what AF suggests there should be done. the other thing is practical nonsense, maintenance wise. not doable.
The reason this is not done yet, is cause i wanted a more sophisticated version checking thing, which would include eg. the engine version, mod name and version and whatnot, but as was discussed in other threads, this would be done otherwise. and as pureint is in master now, i guess the time to implement this is good now.
The reason this is not done yet, is cause i wanted a more sophisticated version checking thing, which would include eg. the engine version, mod name and version and whatnot, but as was discussed in other threads, this would be done otherwise. and as pureint is in master now, i guess the time to implement this is good now.
Re: AI interface version is not used at all
What we want to to here, is check ABI (Application Binary Interface) compatibility.
To do this, we have some stuff prepared in the source already (see eg. aidefines.h). But it is only partly useful, as it can not find all incompatibilities. For example, it would detect if the number of functions in the callback changed, but not if names or arguments changed.
To really find all incompatibilities, we could create a hash over all the headers in rts/ExternalAI/Interface/ in the build-system (CMake), pass it to the source with a define, and use this as a version number. The problem with this is, that it will also detect incompatibility if merely a comment or white-space changed.
What we would need, is a way to create a kind of pre-compiled form of the headers in rts/ExternalAI/Interface/, and do a hash over these. I feel like actual pre-compiled headers is not the right thing here, as it is meant for only a single header, and it does work differently on different compilers. An other method would be, processing/stripping the headers with AWK, and hashing the resulting files. Please give input about how else it could be done.
To consider:
To do this, we have some stuff prepared in the source already (see eg. aidefines.h). But it is only partly useful, as it can not find all incompatibilities. For example, it would detect if the number of functions in the callback changed, but not if names or arguments changed.
To really find all incompatibilities, we could create a hash over all the headers in rts/ExternalAI/Interface/ in the build-system (CMake), pass it to the source with a define, and use this as a version number. The problem with this is, that it will also detect incompatibility if merely a comment or white-space changed.
What we would need, is a way to create a kind of pre-compiled form of the headers in rts/ExternalAI/Interface/, and do a hash over these. I feel like actual pre-compiled headers is not the right thing here, as it is meant for only a single header, and it does work differently on different compilers. An other method would be, processing/stripping the headers with AWK, and hashing the resulting files. Please give input about how else it could be done.
To consider:
- all headers are C only
- we should get the same results with different compilers/build-system
- optional (unimportant): create the same hash on different OS
- stuff like 32 or 64 bit-ness can be checked in an other way -> does not have to be considered in the hash
Re: AI interface version is not used at all
Compiler differences can be overcome by simply using an existing preprocessor tool e.g. http://mcpp.sourceforge.net/
Re: AI interface version is not used at all
the problem is not the pre-processor, but the compiling step.
if the preprocessors of different compilers would spit out different stuff when fed with these headers, that would be a bug in the headers. hiding this in the version number would be bad, so using the preprocessor of the compiler that will be used to compile is preferable, if it need be done.
if the preprocessors of different compilers would spit out different stuff when fed with these headers, that would be a bug in the headers. hiding this in the version number would be bad, so using the preprocessor of the compiler that will be used to compile is preferable, if it need be done.
Re: AI interface version is not used at all
If we run the headers through the mcpp preprocessor on spring current, and the headers the AI used, the output should be identical if both use the same version.
If the headers are identical, then of course the AI must be compatible since the same headers and interface data is being used to compile the ABI, and the interface is based on a C API.
As such, we need only calculate this hash in the spring github repo and bundle it in a separate header file so as not to effect the preprocessor step. That way if the version hash is identical, we know that the source both binaries were built from is the same, regardless of compiler, and thus we are using the same interface version.
We could automate the whole process this way ^_^ I suggest the version ID and the function that implements the callback for getting the version be included in an autogenerated header file, then included in the AI headers surrounded by ifndef guards. These can then be used to filter out this include in the preprocessor stage, and then the resulting header can be hashed, and then the hash written to the header. That way the AIs would not need to be modified, and there would be no accidental releases that have newer interfaces but the version has not been updated.
If the headers are identical, then of course the AI must be compatible since the same headers and interface data is being used to compile the ABI, and the interface is based on a C API.
As such, we need only calculate this hash in the spring github repo and bundle it in a separate header file so as not to effect the preprocessor step. That way if the version hash is identical, we know that the source both binaries were built from is the same, regardless of compiler, and thus we are using the same interface version.
We could automate the whole process this way ^_^ I suggest the version ID and the function that implements the callback for getting the version be included in an autogenerated header file, then included in the AI headers surrounded by ifndef guards. These can then be used to filter out this include in the preprocessor stage, and then the resulting header can be hashed, and then the hash written to the header. That way the AIs would not need to be modified, and there would be no accidental releases that have newer interfaces but the version has not been updated.
Re: AI interface version is not used at all
Not necessarily.AF wrote:If the headers are identical, then of course the AI must be compatible since the same headers and interface data is being used to compile the ABI, and the interface is based on a C API.
If no padding is specified on structs for example, then they may have a different layout on different compilers even if the headers are identical.
Same for the calling convention.
Though IIRC the C interface does not use structs and specifies calling convention explicitly? If those two conditions are fulfilled then it's probably correct.
(Though ABI version would still change by whitespace/comment/ordering fixes in the headers, as hoijui mentioned..)
Re: AI interface version is not used at all
Whitespace and comments can be stripped out after preprocessing using sed
http://stackoverflow.com/questions/2413 ... c-comments
http://stackoverflow.com/questions/2413 ... c-comments
Re: AI interface version is not used at all
i do not see what the pre-processor is useful for in your proposal. we get the same effect when using only awk (we already have awk readily available in CMake on different OS, sed not) to strip out the comments/white-spaces.
as i said, we need to arrive at the same pre-processed headers on different compilers anyway. if this is not the case, it is a bug in the headers and needs to be corrected there. it does not have to be incorporated into a version hash.
basically, what we have two options:
both ways have pros an contras i guess. the later one may show incompatibility where there is none, and the source file approach will likely consist of self-made stripping scripts -> one more source for errors, need to be maintained.
as i said, we need to arrive at the same pre-processed headers on different compilers anyway. if this is not the case, it is a bug in the headers and needs to be corrected there. it does not have to be incorporated into a version hash.
basically, what we have two options:
- reduce the headers (source files) to the extend where they represent only what actually matters for ABI compatibility
- compile them into some binary format
both ways have pros an contras i guess. the later one may show incompatibility where there is none, and the source file approach will likely consist of self-made stripping scripts -> one more source for errors, need to be maintained.
Re: AI interface version is not used at all
BrainDamage pointed me to the ABI_compliance_checker.
This is a perl tool, so possibly not optimal to use from CMake (cause of the perl dependency).
The wiki page also mentions some Similar tools, among them icheck, which seems to be better fit for what we need. Sadly, it looks like it... pretty much died in about 2005 or so :/. It is a C utility that compresses C headers to a form that represents ABI compatibility.
Maybe we can get to latest sources of it, and if it is simple (in terms of dependencies), we might just be able to compile it ourselfs (also for windows) and use that.
This is a perl tool, so possibly not optimal to use from CMake (cause of the perl dependency).
The wiki page also mentions some Similar tools, among them icheck, which seems to be better fit for what we need. Sadly, it looks like it... pretty much died in about 2005 or so :/. It is a C utility that compresses C headers to a form that represents ABI compatibility.
Maybe we can get to latest sources of it, and if it is simple (in terms of dependencies), we might just be able to compile it ourselfs (also for windows) and use that.
Re: AI interface version is not used at all
not able to contact the author of icheck, but i found a source package from debian.. it is perl too. possibly it is even the forefather of the ABI Compliance Checker. Speaking of which, the wiki states it only works on POSIX. though, i cant imagine the part we need would not work on windows, as it just converts C source.
Re: AI interface version is not used at all
am working on a custom made system. quite complex for that small thing it will do in the end. AWK plus CMake magic.
quite some progress made already: can create a file containing an MD5 hash that sums up a customizable list of header files, and gets actualized only & always when it should (eg, headers or scripts change). i can also put it into a #define in a C header, but will now try to get it into the *Info.lua files as a property, so it could also be used by unitsync.
quite some progress made already: can create a file containing an MD5 hash that sums up a customizable list of header files, and gets actualized only & always when it should (eg, headers or scripts change). i can also put it into a #define in a C header, but will now try to get it into the *Info.lua files as a property, so it could also be used by unitsync.