Fixing "broken" HD models

[Uncle]

Hello, does anyone have or know of fixes to "broken" HD models? Particularly when used as Special Effects.

Examples:
Thunderclap plays it's sound multiple times in a row.
Shockwave fails to rotate properly when changing it's Yaw.

I believe I could use Retera's model studio to remove the Sound from the Thunderclap model, but I'm not too sure where to begin for a fix to this Yaw issue:

The picture is a little misleading, the Shockwave effect travels in the correct direction, it's just the effect's Yaw that isn't facing that direction. The Yaw does change though, it's just off the mark most of the time. Also, it's just the white crescent shape that has this issue, the yellow sparks/wave work fine.

It'd be neat if we could create a collection of fixed versions of these models, I just don't fully understand how to do that. Thanks!

 

[Retera]

If we have an imaginary little sit down with Brad Chan at the office and have a look at Engine/Source/Model/MdlParticleEmitterPopcorn.cpp in the MdlReadLoadEmittersPopcorn where we parse the model data, obviously that part's fine because we can cast Shockwave normally and it works. But what's different? We could have a look at War3/Source/Unit/Ability/CAbilityShockwave.cpp and think... what is this doing that you're not doing in your trigger action? Let's have a look at CAbilityShockwave::LaunchMissile and then jump to CMissileShockwave's Initialize getting called on bullet (that stack variable called with the custom CAgentPtr smart pointer stuff).

So, in this case because CMissileShockwave is a derived class that inherits from the base class CMissileCarrionSwarm, we know that we should jump into CMissileCarrionSwarm::Initialize to read what's actually happening.

CMissileCarrionSwarm::Initialize(
        TInitBullet *init,
        NTempest::C3uVector *dst,
        const unreal *range,
        CUnit *targetedUnit,
        int targetFlags,
        CUnit *source,
        const unreal *startRadius,
        const unreal *endRadius,
        const unreal *damage,
        const unreal *totalDamage,
        int weaponType,
        int buffId)

I think one of the keys here is that when CAbilityShockwave::LaunchMissile calls to initialize here, it passes dst as an arg. Knowing the destination allows the model instance spawned into the world to have its yaw on startup, instead of your dynamic yaw manipulation following the construction of model instance. But let's keep reading in Initialize to see if what we find jives with this understanding:

We can easily see that the first line of the function is to call the Base class initialize method: CBullet::Initialize(init, dst); to initialize ourselves as a base CBullet. So we hop into War3/Source/Unit/CBullet.cpp to take a look. Obviously the destination point for the shockwave to launch to is a 3-component unreal vector from tempest library (NTempest::C3uVector *dst argument to function), we call asC3Vector on it to make this a standardized floating point 3-component target location, passing that again into another base class this time called CBulletBase::Initialize(...). And it is there inside of CBulletBase::Initialize(TInitBullet *init, NTempest::C3Vector *dest) that we called the CWar3Image::Initialize function on init->m_image from the TInitBullet in the argument list.

But this points out something I may have overlooked in getting to this point. What is the TInitBullet and how it is able to initialize the CWar3Image (what I would personally tend to call the "model instance") into the correct location? For that, let's back up back out of our current analysis for a moment and have a look at the caller of CMissileShockwave::Initialize. This is a virtual call from the base class CAbilityCarrionSwarm, so we find the call within the code for

CAbilityCarrionSwarm::DoWave(
        CAbilityCarrionSwarm *this,
        const unreal *tx,
        const unreal *ty,
        CUnit *specificTarget)

Inside of this function, we see that we are assigning init.m_image.m_x, init.m_image.m_y, and init.m_image.m_z for the "image" (really the 3D model instance) based off of the caster source and then the Z coordinate from SurfaceCalcAltitude as you would expect. We do the same thing to produce that C3uVector dst for the destination that was getting passed around, again here in the DoWave function, by using the target point of the ability being cast (bounded by world bounds with a call to WorldBoundsUnreal(...)) and a second call to SurfaceCalcAltitude for the Z dimension.

So at this point, if you're following me, it's all about the CWar3Image inside of our TInitBullet and who is telling that thing its yaw in the special way. My hypothesis coming into this is that we might find the yaw is already assigned in the CWar3Image by the time we initialize it, versus your case where we assign it late. But it would be nice to track the specific line of code to be certain.

If we head over to War3/Source/Unit/CWar3Image.cpp and have a look, there are a couple of different method overloads on CWar3Image::Initialize. One of them, for example, is CWar3Image::Initialize(const char *fileName, int addon, unsigned int teamColor) where we are spawning this from the MDX filename, along with an addon setting and a team color.

This points out an interesting case where CWar3Image::Initialize(TInitWar3Image *info, int addon), the overload that we are using, is actually taking in a TInitWar3Image type, rather than an MDX file name. This lets them pass in a bunch more metadata here at creation time. But as we saw in CBulletBase calling to CWar3Image::Initialize(&m_image, m_addon), it's evident that the m_image we set up with the spell source X/Y/Z previously is actually of the TInitWar3Image initializer type and is not the CWar3Image itself. Rather, instead, CBulletBase, as you may have noticed, is a derived type of the image itself.

The CWar3Image itself, if we do some code reading from inside of Initialize, is creating the "sprite" type from the third party libs (Agile/Storm/tempest/etc/whatever via SpriteCreate(info->m_uberSprite)) and wrapping it in this Warcraft 3 style wrapper. So maybe what I said previously (i.e. that a CWar3Image is akin to "model instance") is actually a somewhat incorrect interpretation. If we were comparing this to Warsmash, it would probably moreso be the case that CWar3Image would be akin to UnitAnimationRenderListener stuff and Sprite would be more akin to "model instance" from the 20 years advanced Ghostwolf render system. But once Actiblizzzarions finally publish this code, if we can get them to do that, surely nobody would care about my version. Sorry for tangent.

So, anyway, focusing back on Reforged, we can see in CWar3Image.cpp where we've got it open in the CWar3Image::Initialize(TInitWar3Image *info, int addon) overload, that in this overload after we use CreateSprite we actually then go on to call SpriteSetPosition (related) and most importantly SpriteSetOrientation to actually configure the sprite to be rotated with the yaw (and other data) provided by constructing the 3x3 transformation matrix necessary inside of NTempest::C33Matrix type object, to then call the Rotate utility function of the C33Matrix class so that we can do an axis-angle computation rather than needing to use a quaternion (very convenient to use axis-angle in a case like this). So our situation here -- again for the builtin Shockwave/CarrionSwarm and not for your trigger -- the rotation used for this axis-angle computation is info->m_facing from the TInitWar3Image *info initialization info. It's literally the yaw. So, the Sprite is created, the CWar3Image managing it rotates it and builds transformation matrix off of that, and then we return from this initialize function. And that works great in the melee versus case that Brad Chan cares about, yeah, but it's not working in your case. And what are you doing that is making it not work?

I guess back when your buddies snuck their way into this company and when they worked here before Bobby Kotick managed to lay them off, they had this idea that you want to be able to call SpriteSetOrientation in your map script even though obviously map script was invented for campaign cinematics and high level world events and whatever you're doing is crazy.

And I guess that's how SetSpecialEffectYaw came to be, which we can see in a search is another function who is calling SpriteSetOrientation in much the same way. Of course this function's implementation is different in that it does not use C33Matrix::Rotate with an axis-angle computation to apply the yaw, because the script kiddies over here at Hive are also trying to manipulate pitch and roll. So, the latest and greatest Reforged implementation of this function actually uses C33Matrix::ToEulerAnglesZYX to read out the existing euler angle rotations as best as we can derive them, then overwrites the yaw portion with the float unwrapped from the net-safe machine-independent reliable floating point implementation ("unreal") used for keeping the cinematic campaign scripting events consistent in lock-step multiplayer that we get from unreal::asFloat. Then, lastly, we pack the euler angles back up with C33Matrix::FromEulerAnglesZYX into the completed transformation matrix, use CWar3Image::GetDisplaySprite as a sort of hacky way to pull the Sprite reference back out from under the CWar3Image wrapper, and then call the same SpriteSetOrientation.

So you're probably sitting there and thinking to yourself: if both code pathways lead to the same SpriteSetOrientation call on the 3d model "sprite" instance, how can this go wrong? What is the situation where the rotation does not apply even if we apply it literally the exact same way? Seems like there are two possible candidates while reading this code:
1) Either the CEffectImage constructor/initialization messes up his Sprite in some way, or else
2) The actual Sprite stuff outside of the War3 project and in the second dependent "Engine" code is actually literally broken and SpriteSetOrientation only works the first time in some cases

I would imagine this problem would be much easier to understand if the cause is #1 from above, but again I really like this idea of code reading instead of only ever black-box testing, so let's just literally go read the code and find out!

We can first check War3/Source/Unit/CEffectImage.cpp to see what we can find there. How about the Initialize functions again?

CEffectImage::Initialize(
        CWar3Image *target,
        const char *filename,
        int addon,
        bool persist,
        unsigned int linkIndex,
        unsigned int teamColor,
        const int *attachPropArray,
        unsigned int attachPropCount,
        unsigned int playerID)

When I go and look at this, we can see that it's calling the "fileName" version of the overload for initializing its CWar3Image base class, CWar3Image::Initialize(filename, this->m_addon, teamColor) but I already saw when I was in there earlier that this overload is simpler than the one with the TInitWar3Image extra info, and it doesn't call SpriteSetOrientation. But how could it be that a builtin function that calls A then B, or a user function that calls to a builtin A then a builtin B ends up with a different result? So I second guessed myself a bit here and I started thinking that by jumping to the base type CEffectImage::Initialize I might have been looking at the wrong one. If we open up the AddSpecialEffectById types of stuff, and we walk through your user functions and how they get back to spawning, it seemed to me upon further review that this actually spawns an instance of CEffectImagePos. And all the overloads of CEffectImagePos::Initialize actually call to CEffectImagePos::Initialize_ for their implementation:

CEffectImagePos::Initialize_(
        const unreal *x,
        const unreal *y,
        const unreal *z,
        const unreal *face,
        const char *filename,
        int addon,
        bool persist,
        unsigned int teamColor,
        unsigned int playerID)
{
    // ...
}

And lo and behold down along through this implementation, it's actually even more similar to Shockwave/CarrionSwarm than I was initially giving it credit for:

  this->CWar3Image::Initialize(&info, addon);
  SpriteSetUserFlags(this->m_sprite, 7u, 1);
  if ( this->m_sprite )
    SpriteAnimate(this->m_sprite, 0.0, 0LL, 0);

As we can see here, it calls to the same CWar3Image::Initialize method with a TInitWar3Image type setup, too!

But here's where something really catches my eye: this code pathway includes a call to SpriteAnimate. So, if we were going to trace calls to the 3D sprite, based on all this code reading we would expect:

CAbilityCarrionSwarm and its derived types, hardcoded spell from game:

SpriteCreate(...)
SpriteSetOrientation(...)

Your userland stuff that creates a CEffectImagePos as a proxy for pretending to do a spell:

SpriteCreate(...)
SpriteAnimate(...)
SpriteSetOrientation(...)

So, now based only on code reading and not on the endless Hive trope of black-box testing, we can start to create a hypothesis for what we think might be happening to cause what you described. Namely, it seems that the animations automatically start playing in the case of the userland "effect" natives with this extra call to SpriteAnimate. (A user unfamiliar with the codebase might wonder if/when the CBulletBase stuff ever calls SpriteAnimate. At a quick glance I didn't find where it does, but that likely doesn't matter -- we know that it didn't do so in the code pathway after creating the sprite and before setting its orientation, because all of that was in one function -- and some cursory inspection of the code suggests that any War3Image spawned into the world will eventually have SpriteAnimate called by the World Frame based on whether it's inside of the 3D camera viewing frustum, etc.)

Again, without any black-box testing, we should be able to now test our hypothesis by looking in the code and trying to find a place where the C33Matrix instance is copied at the start of SpriteAnimate and then not copied afterwards which would seem to then explain the difference between these two cases.

So, what does SpriteAnimate do? Based on the behavior of the similarly named Jass natives, when I was trying to rewrite my own remake of this game I made some similar function and it was literally just the thing that ended up "selecting and playing an animation sequence," as we say. But, I have to say, when I went to read the code for this function, I was not impressed. It's easy enough to head over to Engine/Source/Services/Sprite.cpp and have a look, but, Brad Chan and the guys over there who leaked the debug symbols, you know, didn't actually leak the source code and so it might help for me to add some clarity here to what I was doing thus far. This post has been written from a tiny raspberry pi Linux computer while I am on a vacation outside of my home country, in some other place. This machine does not have Reforged installed, it does not have the correct processor architecture to even try to play Reforged, but it does have a text editor and some access to basic information about what leaked in December. And it's based on that information that I am over here pretending to be looking at the source code, pretending how this would go if I were there in the office with Brad Chan. I am, of course, not affiliated with Activision Microsoft in any way.

As such, when I went to look at what SpriteAnimate actually does, this garbage that I'm looking through actually gives me this:

  if ( sprite )
    return (*(__int64 (__fastcall **)(HSPRITEBASE__ *, __int64, HCAMERA__ *, _QWORD, _BYTE))(*(_QWORD *)&sprite->HDATAMGR__
                                                                                           + 24LL))(
             sprite,
             v4,
             camera,
             cameraIndex,
             0);

This is the sort of code that entices human beings to use programming languages other than C/C++. The code as written has a meaning, but it is a meaning that may take someone a great deal of time to understand. It is also not the original code from the Blizzard office. This seems to be a case where the intelligence of whatever machine attempted to understand the debug symbols, and their actual meaning, has still diverged. Assuredly what is actually happening here is that we are calling a function pointer who is a member variable of the Sprite type defined in Engine/Source/Services/Sprite.h, but whatever created this thing in front of us did not know that.

I would like to continue, and maybe I will, but I'm hitting a wall where I shall have to sleep for a period of time. So, I will leave it as an exercise for the reader to read through SpriteAnimate and where we get into trouble here. But, based on my knowledge of this codebase as a black-box tester such as yourself, I can tell you what I think the problem is and I can suggest a solution than I think has a high percent chance of success.

A lot of times when I try to use every available resource like this to do an accurate write-up of a problem on Warcraft III, someone will end up linking this accurate write-up to a guy such as Brad Chan and then this guy, my guy the temporary employee, he will ruin the game worse than it already is by trying to solve the problem. As an example of that, maybe he will read this far in my post and go and delete the SpriteAnimate call inside of CEffectImagePos::Initialize_(...) and hope that it might solve the problem, pushing it into production without testing all affected edge cases. Because, indeed, how could he test all edge cases? For a temporary employee, to do so is likely inconceivable.

I have a hunch that whoever added SpriteAnimate here -- even when I do not have the git blame stuff -- was probably a very smart person who was also trying to fix a legitimate user problem. And when we take it away, some other edge case is going to break horribly. If this was Warsmash, my rewrite of the game, when we would have this problem -- and we did have nearly exactly this problem as it pertains to the userland scripts for adding and removing secondary animation props, and the desire to write userland (and thus moddable) versions of abilities -- where it turned out that emulating the old C++ code means being able to add and remove secondary animation props without triggering a new call to SpriteAnimate, but to properly mimic the userland existing stuff we do need to trigger a call. So, what I'm saying is, in my case I solved this for my engine developers by saying that when they rewrite all the ingame abilities in JASS, they can use new natives that I added which are the same as the old ones but omit the call to SpriteAnimate. And, back then you know everything I did was clean room engineered so we didn't call it SpriteAnimate because I didn't think of that, but I did add a native to do exactly that same thing, basically. Anyway, to conclude that thought, if this was my own game engine I would make the API better so that you could essentially choose to call SpriteCreate then SpriteSetOrientation then SpriteAnimate, only, in that order, as your userland scripts. But that's not Reforged, you know. Reforged is going to be used by Johnny the World Editor User to try to play his backup of his old RPG from 2011 that he spent five years making, and when it breaks and the animations don't SpriteAnimate, he will not know what that is and he will get just go on social media and complain how Activision Microsoft has hollowed out Blizzard's soul. So, if Brad Chan ever reads this writeup here -- maybe after someone links it to him for the memes -- he can't easily give you a better scripting API because he doesn't have the funding and money and because of those aforementioned issues, etc, and he also can't edit the existing API to avoid the SpriteAnimate call. So what can he do?

Maybe he can't do anything yet... Mr @Uncle . Maybe his hands are tied, and this one is up to you. I have to sleep but it would be very nice if you could utilize the information that I presented here and come up with a solution. Because based on my black-box testing knowledge -- and now I'm speculating, and not reading code when I say this -- I think the problem is (like a number Reforged problems) likely caused by the PopcornFX particle system integration. Because the integration of that... in my opinion... is bad. Warcraft III was its own render art technology, you know? The CWar3Image stuff and Sprite stuff is a lost art, and nobody online had the code for how that works. And then the 2020 guys had this idea to just shovel into that code a link to proprietary code from PopcornFX company -- code that the guys at Activision probably cannot even sit and read and review to try to understand themselves -- which is a second render engine similar to all the Warcraft III 2003 stuff (which despite a few changes to add the shiny metal shader enhancements of Reforged is largely the same). So there are two render engines. And all that stuff I was reading through above stops being relevant the moment that Engine 1 calls to SpriteAnimate and plays the animations and triggers it to load in the "Popcorn" particle emitters, which then render in "Engine 2" in a shared 3D output. And I would sure guess... although again this is speculation... that after the 2 different render engines each have their model object spawned (the MDX object in the War3 engine, and the PKB object in the Popcorn engine) that the limited extent to which they talk to each other does not include re-copying that C33Matrix (which is notably a "tempest" class from the proprietary Blizzard code) into whatever format of transformation matrix is expected by the proprietary PopcornFX code. Since each PopcornFX model contains its own node scene graph of shader programming, it might even be the case that Shockwave literally reads from the transformation matrix and stores it into a variable inside of war3.w3mod:_hd.w3mod:Abilities/Spells/Orc/Shockwave/ShockwaveMissile.pkb (which notably is NOT the MDX model) and we would never see that when reading the C++ source code like I was pretending to do above! Because in that case, the "bug" that you want a solution for would literally be in the PopcornFX Editor UI in the Scene graph used to compile this one specific asset! But what are you going to do about that? Are you going to make a new asset that does not have that problem? How are you going to do that? Will you convince Brad Chan and his friends to finally cough up the uncompiled pkfx raw assets for you to play with in the PopcornFX Editor program? It's true that PopcornFX editor exists as a tool that any of us can download from the PopcornFX website. But it is useless without the original Shockwave source file. Or will you download that program and reinvent your own version of Shockwave just so that you can fix 1 tiny thing?

Brad is not going to do it. I think that you are not going to be able to convince him that it is worth his time. He's more of an AI-upscaled-classic-graphics kind of guy, judging by the patches they released. So what are you going to do?

I think that if I were you, I would extract the ShockwaveMissile.mdx file -- because we have much better MDX editors these days than PKB editors -- and try to modify it so that when SpriteAnimate is called on it the first time, the ParticleEmitterPopcorn ('CORN') node in there does not display the linked PKB model in the default animation selected by the first call to SpriteAnimate. I think you can probably do this by giving it a Stand animation with =off in the Anim Visibility Guide thing. Then, again if we imagine tracing calls to the Agile/whatsit Sprite class from your gameplay, it might look like this:

SpriteCreate(...)           // -------- triggered by AddSpecialEffectByWhatever jass/lua native
SpriteAnimate(...)      // -------- triggered by AddSpecialEffectByWhatever jass/lua native (even though you didn't want it, you can't turn it off) and it chooses "Stand" animation which is invisible
SpriteSetOrientation(...)          // -------- triggered by BlzSetSpecialEffectYaw or BlzSetSpecialEffectOrientation native of your choice
SpriteAnimate(...)      // -------- triggered by BlzPlaySpecialEffect jass/lua native (and you must trigger an animation different from the default stand, wherein your ParticleEmitterPopcorn link uses `=on` in the MDX now)

As far as the black-box testing of casual style Warcraft 3 modding goes, if you configure the system in the manner described above, I would be very interested to know if this works. We could go code-surfing in C++ again and try to find whether the "Anim Visibility Guide" stuff creates the Popcorn node in the Popcorn render engine even when it is in the =off state -- I do not particularly know. But it is most likely faster to just test it black-box, because I have to sleep. And as mentioned previously, unfortunately I am on a computer that is not capable of playing Reforged and so I am not easily capable of testing this for you. If you try this, and it does not work, there are still a few other avenues to pursue before giving it up and leaving it to Brad Chan to just make it better some day magically. One example that comes to my mind, which again is a very stupid problem to even exist is that my understanding is that SpriteCreate can create a PKB-only sprite that isn't an MDX, even though nothing in the game does so. It seems to be a code change that was introduced in Reforged maybe as a test, before the developers eventually concluded that the only time we should ever spawn a PKB instance into the scene was with an MDX wrapper holding its hand and telling it what C33Matrix to use, etc. Because of that extremely weird legacy code, and because war3.w3mod:_hd.w3mod:Abilities/Spells/Orc/Shockwave/ShockwaveMissile.mdx and war3.w3mod:_hd.w3mod:Abilities/Spells/Orc/Shockwave/ShockwaveMissile.pkb have the same file path other than their file extension, and because Warcraft III will sometimes discard file extension in the hopes of "guessing" that you might have map code from a Debug build with .mdl or .pkfx in the file, but you might be playing on a Release build with .mdx and .pkb binaries, as a result you can sometimes think you are loading an MDX file that gets passed to SpriteCreate but instead you end up creating a PKB-only thing. And usually if that happens, it is unconscionably bugged. If Brad Chan reads this, and he's looking at the implementation of SpriteCreate and thinking about what I'm saying, he should definitely go in there and change it so that PKBs can only be spawned by the Engine services themselves in response to a PopcornEmitterParticle ('CORN') wrapper and not from a call to SpriteCreate to the PKB's own file path. IN MY OPINION. Then again, maybe there was some Reforged campaign mission that utilized the direct-spawn function in a cinematic, and I didn't know because I didn't spend a lot of time looking at that. If so, then maybe just don't allow .mdl file path to guess .pkb, and don't allow .pkfx filepath to guess .mdx ? Seriously?

So, because of all that hot mess, if you try to create a custom model with an invisible Stand animation and then use the new Jass natives to orient it correctly, and ONLY then play its animations -- and if all that idea doesn't actually work -- then you could try this second idea of creating an MDX file of shockwave with a unique file path separate from its PKB, and try spawning that, to ensure you were not hitting the bizarre "pkb only" case in the userland code somehow.

If neither of those two things work, then.... I don't know... maybe some other person will reply on here with a better idea (lol?). I must sleep.

 

[Uncle]

I appreciate the massive breakdown. Admittedly, it's somewhat wasted on me, but I get the gist of it.

I'll look into those two potential solutions. Although, I'm tempted to just concede and use alternate models or request some new models from the lovely modelers at Hive. Thanks!

 

[Kakerate]

cinema