Hi. I’m Retera.
If your search the source control in the .cpp files for the string MDLKEY_KMTF, and then on those section(s) of the C++ code have a look at the “Git Blame” or equivalent.
You’ll notice that in the Patch 1.33 changes that you’re pushing live on August 17th, 2022 you’re now releasing a change to this section of the model loader subroutines. The change is from the previous team, but was not ever released to us in the public before. It should show in the “Git Blame” or equivalent as some new code.
There is a bug in this code. The scope of affected use cases involves Classic graphics, such as the Arch Mage’s summoned Water Elemental.
Previously – if you look prior to the 1.33 changes – in the switch case when the C++ code encounters the MDLKEY_KMTF, then we were calling ReadBinIntKeyFrames where the first reference passed into the function (the destination, to read into) was a class member variable on the material’s layer.
But in your latest code, including the changes that you are releasing tomorrow, you will see the call to ReadBinIntKeyFrames is now more complicated. The first function argument – the destination reference to read into – is now one of several layer components, because HD layers are composed of multiple textures on the Patch 1.33. I do not know why this was changed, but the reasoning for the change is probably included in your issue tracker subsystems. However, rolling back the change would be too complicated so I suggest keeping this new Patch 1.33 solution and simply fixing the bug in it.
So, now let’s talk about what the bug is. I do not have your code, so I can only help you with this to a certain point to describe the way the issue manifests to the end user. The problem for the end user seems to be that the MDLKEY_KMTF branch is calling ReadBinIntKeyFrames to read into a structure that is unused sometimes, when instead the intention would have been to use the data output into that first argument reference.
The difference on the “Classic” graphics arch mage Water Elemental, if you compare the way the model looked on the old game and the way it will look after your patch tomorrow, is that the watery part of this character will NOW appear to slide downward constantly but have no other texture ripples on the surface. However, in the game up until tomorrow, the appearance of this 3D model was a flipbook of textures constantly swapping AND sliding downward. The difference is because the MDLKEY_KMTF data was loading successfully until the Patch 1.33 changes for classic graphics on this 3D engine.
As a bit of background, the Patch 1.33 changes that you are pushing into production changed the way that MDLKEY_KMTF is used so that one “Layer” construct can load MDLKEY_KMTF binary chunks multiple times into different parts of the layer. Previously, on Patch 1.32 and in all previous versions of this format, the MDLKEY_KMTF call to ReadBinIntKeyFrames always had the same layer.whatever argument passed to the read destination (first argument).
However, as a note, fixing the bug is NOT AS SIMPLE as reverting the first argument passed to the ReadBinIntKeyFrames to be the one-for-all state container that it would have been previously prior to these changes. An example model to make sure you move forward and do not cause regression is the “Reforged” graphics model for the Enraged Elemental (‘nele’) creep unit type, which uses the file path War3.w3mod\_HD.w3mod\Units\Human\EnragedElemental\EnragedElemental.mdl or something close to this (I do not know if the file path in the office exactly matches what we see on the outside after the build process). In particular, this is a Reforged HD model that needs to use MDLKEY_KMTF multiple times for a single layer, so this model would become visually distorted if your attempts to fix the “Classic” graphics Water Elemental break the HD use case for why this code has now changed on Patch 1.33.
Hopefully this information is enough for you to go off of. I am bringing up this issue not because I care about the amount of ripples on the Classic Water Elemental, but actually because this engine-level parsing issue (that causes some parsed data to fail to load, or to be omitted in favor of static TextureID when it was not previously static for these assets, or whatever the underlying bug is) is an issue affecting many other use cases in the “Classic” graphics mode. And the issue is new on Patch 1.33.
I am sorry that I do not have more specific information yet about where the bug is in the C++ decision to use, or to skip over, the results of loading the MDLKEY_KMTF chunk. There are clearly some Classic models that are parsing without any visible exception or failure, then omitting their MDLKEY_KMTF information even though all of the new Reforged HD models seem to load correctly – and some Classic models seem to still load correctly. The new Patch 1.33 HD-focused multi-inputs to the Layer that read repeated MDLKEY_KMTF chunks into their corresponding parts appear to be able to sometimes load the Classic information correctly and other times erroneously discard the Classic graphics information of these chunks. The bug depends on the model structure, but I have not quite figured out (with black box testing here on the outside) what the bug is yet.
Another model file that is easy to test and is more obviously broken than the Water Elemental summon of the Arch Mage, would be to test the Classic graphics model for the Spawning Grounds building in the Naga race. This building has a pool of water in the center, but on Patch 1.33 there is only blackness (or visual artifact) in the center where water would have been located. This is because the Layer chunk is erroneously loading its static TextureID field for that model instead of the MDLKEY_KMTF data that would have visually appeared like water.
[NOTE: My information regarding the contents of your C++ code may be based on wild conjecture, rumor, or other incorrect information. However, at the end of the day, there is no doubt as a user that this model parser (or the application use case of the parsed data) has a bug.]
Edit/Footnote:
While reviewing the Warcraft III model parser (please don’t break it!) you may notice that the code will sometimes branch on a variable named version or possibly FormatVersion. Based on my empirical evidence and tests, all of the possible format versions are visually affected by this issue and the bug is not version specific. I do not know if this helps you or not to know that, but I thought it might be worth mentioning.