Script errors with 4.14.0

[Enjay]

[Edit: With help from the community, the reason for the changes have been explained and the fixes that I needed to get my projects working again were provided.

Reason for the changes: viewtopic.php?t=79997 further exemplified by Player701 below
m8f's fixed Gearbox mod: Release page on GitHub forum thread: viewtopic.php?t=71086
Player 701's fix for the PDA code: viewtopic.php?p=1257238#p1257238 forum thread for the original PDA kit viewtopic.php?t=65849

Thank you to those who provided insight and explanations, and for providing what I needed (and what others will need) to get things working again. ]

Original Post...

---

I have just downloaded and installed 4.14.0 and when I start the game, I get some ZScript errors with some mods I've been using. It concerns two mods in particular: Nash's PDA handler and m8f's Weapons Gearbox. Both are possibly older versions of these mods, because they have been in use on my HD for years now. I have modified the files, but I'm pretty sure that I have not modified anything in these particular areas.

Like I said, these have been working for years - right up to version 4.13.2, but they have broken under 4.14.0. The game refuses to start and gives me the error messages listed in red below.

I'm assuming it's something that needs to be fixed in the scripts, rather than the engine, so I'm posting here rather than in the bugs forum.

Unfortunately, the errors really don't mean much to me, so any help unpicking them would be appreciated.

I added the line numbers myself. They're not in the code. The relevant files are also attached. [Removed because they are no longer needed.]

Errors with the PDA handler

LoadActors: Load actor definitions.
GScript error, "00BaseControlText.pk3:zscript/pda/pdamenuhandler.zc" line 112:
GCannot convert Pointer<Class<PDAColoredButton>> to Pointer<Class<PDAOwnedPDAButton>>

GScript error, "00BaseControlText.pk3:zscript/pda/pdamenuhandler.zc" line 113:
GUnknown identifier 'Index'

Code: Select all

111			// Update current selection
112			int index = menu.pdaList.items.Find(PDAColoredButton(caller));
113			menu.pdaList.currentSelection = index;
114			menu.pdaList.UpdateSelection();

Errors with the Gearbox

GScript error, "ZZGearBox.pk3:zscript/gearbox/weapon_menu.zs" line 279:
GCannot convert SInt4 to TextureID

GScript error, "ZZGearBox.pk3:zscript/gearbox/weapon_menu.zs" line 301:
GCannot convert SInt4 to TextureID

Code: Select all

264  private ui
265  void fillDirect(out gb_ViewModel viewModel)
266  {
267    uint nWeapons = mWeapons.size();
268    for (uint i = 0; i < nWeapons; ++i)
269    {
270      let aWeapon = Weapon(players[consolePlayer].mo.findInventory(mWeapons[i]));
271
272      if (aWeapon == NULL)
273      {
274        if (mOptions.isPositionLocked())
275        {
276          viewModel.tags        .push("");
277          viewModel.slots       .push(mSlots[i]);
278          viewModel.indices     .push(i);
279          viewModel.icons       .push(-1);
280          viewModel.iconScaleXs .push(-1);
281          viewModel.iconScaleYs .push(-1);
282          viewModel.quantity1   .push(-1);
283          viewModel.maxQuantity1.push(-1);
284          viewModel.quantity2   .push(-1);
285          viewModel.maxQuantity2.push(-1);
286        }
287        continue;
288      }
289
290      if (isHidden(aWeapon.getClassName())) continue;
291
292      if (mSelectedIndex == i) viewModel.selectedIndex = viewModel.tags.size();
293
294      viewModel.tags.push(aWeapon.getTag());
295      viewModel.slots.push(mSlots[i]);
296      viewModel.indices.push(i);
297
298      TextureID icon = getIconFor(aWeapon);
299
300      // Workaround, casting TextureID to int may be unreliable.
301      viewModel.icons.push(int(icon));
302      viewModel.iconScaleXs.push(aWeapon.scale.x);
303      viewModel.iconScaleYs.push(aWeapon.scale.y);
304
305      bool hasAmmo1 = aWeapon.ammo1;
306      bool hasAmmo2 = aWeapon.ammo2 && aWeapon.ammo2 != aWeapon.ammo1;
307
308      viewModel.   quantity1.push(hasAmmo1 ? aWeapon.ammo1.   amount : -1);
309      viewModel.maxQuantity1.push(hasAmmo1 ? aWeapon.ammo1.maxAmount : -1);
310      viewModel.   quantity2.push(hasAmmo2 ? aWeapon.ammo2.   amount : -1);
311      viewModel.maxQuantity2.push(hasAmmo2 ? aWeapon.ammo2.maxAmount : -1);
312    }
313  }

GScript error, "ZZGearBox.pk3:zscript/gearbox/inventory_menu.zs" line 98:
GCannot convert SInt4 to TextureID

Code: Select all

91      if (item.bInvBar)
92      {
93        string tag  = item.getTag();
94        int    icon = int(BaseStatusBar.getInventoryIcon(item, BaseStatusBar.DI_ALTICONFIRST));
95        viewModel.tags        .push(tag);
96        viewModel.slots       .push(index + 1);
97        viewModel.indices     .push(index);
98        viewModel.icons       .push(icon);
99        viewModel.iconScaleXs .push(1);
100       viewModel.iconScaleYs .push(1);
101       viewModel.quantity1   .push(item.maxAmount > 1 ? item.amount : -1);
102       viewModel.maxQuantity1.push(item.maxAmount);
103       viewModel.quantity2   .push(-1);
104       viewModel.maxQuantity2.push(-1);
105
106       ++index;
107     }

[Enjay]

For what it's worth, I just tried a few git builds.

gzdoom-x64-g4.14pre-120-gfb660fa3c.7z
from December 05 does not throw up the errors and appears to work.

gzdoom-x64-g4.14pre-121-g325817afd.7z
from December 13 does throw up the errors and the game cannot start.

[inkoalawetrust]

This is because Jay pushed a fix for ZScript that makes arrays more strictly obey what type they're supposed to be. For Gearbox at least, there was a discussion on the ZDoom Discord where the cause of the error seemed to be that m8f was using a TextureID array by passing actual plain integer values into it, hence the "Cannot convert SInt4 to TextureID" errors.

I don't think much can be done about it besides updating the mods to be coded better. Since AFAIK Jay did try to make the array fix only apply to 4.14.0+, but that'd just break more things.

[Enjay]

Thanks for the context and explanation. I guess I was right to think along the lines of "it's a problem that needs to be addressed in the scripts rather than a bug."

The downside is that the fixes are beyond me. So I'm trapped in older versions of GZDoom for what I was working on until I can get it resolved somehow. Tying a mod to a specific old version is never a good policy IMO. So, I hope I can get something worked out somehow.

Shame it's not something that could be versioned to pre 4.14.0 I know a handful of mods used the PDA kit, and quite a few more used the Gearbox. So, I guess they will be broken on new GZDoom. However, it does sound like it was a considered decision.

[Player701]

IMO, with a breaking change like this one, the devs should've made an announcement before the 4.14 release, providing at least a couple relatively simple ZScript examples that worked in older versions but would break in the next release. It would've been much easier to understand what is going on here, as I still don't quite grasp it entirely except for the fact that it has something to do with more strict type checking. Fortunately, at least my own project does not seem to be affected.

[Nash]

I'll get around to updating my PDA mod eventually. No time frame, except "as soon as possible". Sorry for the inconvenience.

That said, the engine fixes are absolutely needed and is non-negotiable, so I'm on board with the rest of the team. Leaving code exploits open is never a good thing.

[Enjay]

I really appreciate that Nash. I look forward to the fix as and when you have time for it.

I've been around long enough to know that a mod-breaking change in code would not have been a decision taken lightly. Usually even quite extreme steps are taken to ensure compatibility. So, I'm quite content that the decision was made for very good reasons.

[Player701]

Since no one apparently bothered to provide any example code, I've had to conduct my own investigation. Here's something I've managed to come up with so you can see why the breaking change had to be made:

Code: Select all

version "4.10"

class TestInv : Inventory
{
    Array<class<Actor> > ar;
    
    override bool Use(bool pickup)
    {
        voidptr evil = new('Object'); // Or anything else, really
        ar.Push(evil); // Illegal in ZScript >= 4.11 OR GZDoom >= 4.14
        
        class<Actor> cls = ar[0];
        Spawn(cls, (0, 0, 0)); // This causes GZDoom to crash
        ar.Clear();

        return false;
    }
}

Running this code and typing give testinv and then use testinv will typically cause GZDoom to crash.

What happens here is that we are able to coerce the ZScript engine to treat an Object as if it were a class<Actor> - which obviously causes all sorts of unexpected behavior when we try to peform any kind of operation on it, like attempting to spawn this "actor" in the level. In this particular example, GZDoom will simply crash, but I suspect that instead of new('Object'), it is possible to provide a pointer to some carefully constructed object instance that might result in arbitrary code execution when operated upon in a specific way.

Now I wouldn't imagine an actual gameplay mod to contain any such code, so why did some mods still break when this exploit was fixed, you might ask? In Enjay's case, there are two particular scenarios affected by the fix. The first one:

Code: Select all

version "4.10"

class Base
{
}

class Derived : Base
{
}

class Test
{
    Array<Derived> ar;

    void Demo(Base b)
    {
        ar.Find(b); // Illegal in ZScript >= 4.11 OR GZDoom >= 4.14
    }
}

Do you see the problem here? We have an array of Derived, but we provide a pointer to Base as an argument to Find. This is technically illegal because a pointer to Base is not guaranteed to also point to Derived, although it won't cause any issues at run-time because the contents of the array are not modified - not that the compiler actually cares about this any longer, so the above code is now invalid regardless of the ZScript version being used. Note that replacing Find with Push (or Insert, for that matter) will error out even in older versions - there is special handling for these methods in the compiler in case the array contains object pointers. But what about other types? That's what the second example is about:

Code: Select all

class Test
{
    Array<TextureID> ar;
    
    void Demo()
    {
        ar.Push(12345); // Illegal in ZScript >= 4.11 OR GZDoom >= 4.14
    }
}

From a practical standpoint, there is really nothing wrong here because a TextureID is, in fact, just a number. But from the type system's point of view, a number and a TextureID are quite different things, so this code is not supposed to be valid ZScript - it only used to work due to an oversight. This particular scenario has already been discussed previously in this very thread, so there is not much else here to say.

[Enjay]

Presumably, in 4.14.0 your code examples would just cause GZDoom to refuse to run, citing the specific lines of code as problematic, whereas, previously, they would have been allowed through?


In other news, m8f has released a fix for the Weapons/Inventory Gearbox so it now works with 4.14.0. I'm sure that will be much appreciated by many.

So, from my perspective, that only leaves the problem with the PDA code.
It seems only to be in the area that I quoted in my first post that is problematic, but I don't know how significant a re-write would be needed to fix it. From my uninformed position, it could be anything from a simple change of data type to a complete re-write of the entire thing. (And, I guess, perhaps fixing one thing may cascade to other things needing fixed too.)

So, can anyone suggest something that I might be able to do to to correct it in the meantime? It would allow me to get on with working on the latest game version instead of keeping an old exe around for the sake of this one problem. I know that Nash will look at the PDA kit in due course (which is absolutely fine, much appreciated and there is no implied pressure at all), but if I can get things working with a tweak to the old code in the short-term, that would be useful.

[Player701]

whereas, previously, they would have been allowed through?

Correct, but only with ZScript version 4.10 or below. Previously, the fix was conditionally applied only for ZScript 4.11 and above, but starting with GZDoom 4.14, the fix is applied unconditionally regardless of the ZScript version declared.

So, from my perspective, that only leaves the problem with the PDA code.

I found the PDA mod code on Github while examining the issue. The problem is with the type of argument given to Find:

Code: Select all

int index = menu.pdaList.items.Find(PDAColoredButton(caller));

Here, menu.pdaList.items is of type PDAOwnedPDAButton. However, the argument is cast to PDAColoredButton, which is the parent class. Exactly the same as the example scenario with Base and Derived above.

To fix this, try replacing the cast as follows:

Code: Select all

int index = menu.pdaList.items.Find(PDAOwnedPDAButton(caller));

[Enjay]

I found the PDA mod code on Github while examining the issue. The problem is with the type of argument given to Find:

Code: Select all

int index = menu.pdaList.items.Find(PDAColoredButton(caller));

Here, menu.pdaList.items is of type PDAOwnedPDAButton. However, the argument is cast to PDAColoredButton, which is the parent class. Exactly the same as the example scenario with Base and Derived above.

To fix this, try replacing the cast as follows:

Code: Select all

int index = menu.pdaList.items.Find(PDAOwnedPDAButton(caller));

Marvellous! Thank you very much. With that change (and with the fixed Gearbox) I can get GZDoom to start. On a quick check, the PDAs that I have defined seem to be working perfectly. Very much appreciated, thank you again.