jedidia
shoemaker without legs
As some might already have noticed, there were two compatibility problems between D3D9 client and IMS. The problems were not very IMS-specific and might crop up in connections with other projects as well.
I have tracked down the problems and made a fix. Since I'm not an adept user of version control systems and code repositories, the easiest way seems to post them here. They are not large fixes and easy to implement.
The first problem was D3D9 client writing out of bounds when animations were dinamically created during sim-time.
D3D9 client initialises animations and allocates memory to store the state of those both at loadup, and dinamically when a mesh is added.
This happens in the function InitAnimations(UINT) in vVessel.cpp.
The trouble is, this function only is called when a mesh is added. nanim receives the number of animations the vessel currently has, but this is before new animations are added.
IMS (and presumably any other add-on that creates animations on the fly) then goes on to add the new animations if neccessary. After doing so, vVessel::Update() is called in the client, which calls UpdateAnimations(UINT = -1).
The new animations will have been added in the meantime. Meaning, nanim will receive the new number of animations, while InitAnimations only got the old number of animations, without the newly added ones.
As a result of this, UpdateAnimations writes over the allocated length of animstate in the block at the end.
I fixed the problem by introducing a new block into UpdateAnimations that checks for newly added animations and calls InitAnimations for them. The block seems a bit overcomplicated. The problem is I need the meshindex, and at this point (called from Update()) The meshindex passed to UpdateAnimations is invalid, so I have to extract all meshindices first, and of course I had to check for duplicity, since I don't want to call InitAnimations over and over for the same mesh. I used a vector, which means I had to include vector.h in vVessel.cpp:
I'm probably breaking a convention on every line I wrote here, and I don't know if the inclusion of vector is welcome, as there seems to be an avid evasion of both string and vector in the whole client. I could have made a fixed size array with a size that seems large enough, but I'd rather play it save... :shifty:
It is also possible that it would be sufficient to call InitAnimations just once, even if more than one mesh was added (doesn't happen in IMS, so I didn't test)... as far as I can see it wouldn't make much of a difference, but the function requiring a meshindex made me suspicious.
The other problem had to do with classnames longer than 62 characters causing a crash in ParseSkins() (also in vVessel.cpp). Since the path is included in the classname, this can happen faster than one might think...
The fix was to simply increase the buffer size to 256. This is still potentially unsave, but 256 seems a reasonable enough value and equals the allocated size in pSkins, so extending it further or making the allocation dynamic would require some more involved changes.
The crash is caused by sprintf_s invocing the default invalid parameter handler (which seems to crash the program on purpose to tell the programmer that there's something wrong) as soon as the the string (including terminator) is equal to the buffer size.
I have tracked down the problems and made a fix. Since I'm not an adept user of version control systems and code repositories, the easiest way seems to post them here. They are not large fixes and easy to implement.
The first problem was D3D9 client writing out of bounds when animations were dinamically created during sim-time.
D3D9 client initialises animations and allocates memory to store the state of those both at loadup, and dinamically when a mesh is added.
This happens in the function InitAnimations(UINT) in vVessel.cpp.
The trouble is, this function only is called when a mesh is added. nanim receives the number of animations the vessel currently has, but this is before new animations are added.
IMS (and presumably any other add-on that creates animations on the fly) then goes on to add the new animations if neccessary. After doing so, vVessel::Update() is called in the client, which calls UpdateAnimations(UINT = -1).
The new animations will have been added in the meantime. Meaning, nanim will receive the new number of animations, while InitAnimations only got the old number of animations, without the newly added ones.
As a result of this, UpdateAnimations writes over the allocated length of animstate in the block at the end.
I fixed the problem by introducing a new block into UpdateAnimations that checks for newly added animations and calls InitAnimations for them. The block seems a bit overcomplicated. The problem is I need the meshindex, and at this point (called from Update()) The meshindex passed to UpdateAnimations is invalid, so I have to extract all meshindices first, and of course I had to check for duplicity, since I don't want to call InitAnimations over and over for the same mesh. I used a vector, which means I had to include vector.h in vVessel.cpp:
Code:
void vVessel::UpdateAnimations (UINT mshidx)
{
double newstate;
UINT oldnAnim = nanim;
nanim = vessel->GetAnimPtr(&anim);
if (nanim>0 && animstate==NULL) {
LogErr("[ANOMALY] UpdateAnimations() called before the animations are initialized. Calling InitAnimations()...");
InitAnimations();
}
if (nanim > oldnAnim)
//animations have been added without being initialised
{
std::vector<UINT> meshesToInitialise;
for (UINT i = oldnAnim; i < nanim; ++i)
//extracting meshindices for uninitialised animations.
//this might seem overly complicated, but it seems savest
//it guarantees that all new animations are initialised, and are initialised only once
{
for (UINT j = 0; j < anim[i].ncomp; ++j)
{
bool alreadyNoted = false;
for (UINT jj = 0; jj < meshesToInitialise.size(); ++jj)
{
if (anim[i].comp[j]->trans->mesh == meshesToInitialise[jj])
{
alreadyNoted = true;
break;
}
}
if (!alreadyNoted)
{
meshesToInitialise.push_back(anim[i].comp[j]->trans->mesh);
}
}
}
for (UINT i = 0; i < meshesToInitialise.size(); ++i)
{
InitAnimations(meshesToInitialise[i]);
}
}
for (UINT i = 0; i < nanim; i++) {
if (!anim[i].ncomp) continue;
if (animstate[i] != (newstate = anim[i].state)) {
Animate(i, newstate, mshidx);
animstate[i] = newstate;
}
}
}I'm probably breaking a convention on every line I wrote here, and I don't know if the inclusion of vector is welcome, as there seems to be an avid evasion of both string and vector in the whole client. I could have made a fixed size array with a size that seems large enough, but I'd rather play it save... :shifty:
It is also possible that it would be sufficient to call InitAnimations just once, even if more than one mesh was added (doesn't happen in IMS, so I didn't test)... as far as I can see it wouldn't make much of a difference, but the function requiring a meshindex made me suspicious.
The other problem had to do with classnames longer than 62 characters causing a crash in ParseSkins() (also in vVessel.cpp). Since the path is included in the classname, this can happen faster than one might think...
The fix was to simply increase the buffer size to 256. This is still potentially unsave, but 256 seems a reasonable enough value and equals the allocated size in pSkins, so extending it further or making the allocation dynamic would require some more involved changes.
The crash is caused by sprintf_s invocing the default invalid parameter handler (which seems to crash the program on purpose to tell the programmer that there's something wrong) as soon as the the string (including terminator) is equal to the buffer size.
Code:
void vVessel::ParseSkins()
{
char classname[256];
D3D9Client *gc = scn->GetClient();
DWORD start = 0;
sprintf_s(classname, 256, "#%s", vessel->GetClassNameA());
).
