FourCC breaks in V1.31

[Tasyen]

This a nasty bug happening only in the not upto date Warcraft 3 V1.31. And only for maps in Lua mode. After the game was Saved&Loaded FourCC stops working. This breaks any Unit/Ability/Item/Buff/Destruct/Terrain... Type comparer in GUI and written code.

This does not happen in the upto date version of warcraft 3 only on the old 1.31.

Thankfully Zed (discord) posted a working FourCC code. That one could just add to the map's script to stop the Save&Load Bug.

function FourCC(id)
    return 0x1000000 * string.byte(id:sub(1,1)) +
             0x10000 * string.byte(id:sub(2,2)) +
               0x100 * string.byte(id:sub(3,3)) +
                       string.byte(id:sub(4,4))
   end
  
function GetFourCC(num)
    return string.pack(">I4", num)
end

The map contains 3 GUI triggers. On Player Red selection each checks for an unitType and then prints the unit's name. When the Fix is disabled the triggers will fail to work after the game was Saved&Loaded.

 

[MyPad]

This is an interesting bug (for wc3 1.31, at least). What causes FourCC to fail when a game is saved and loaded? Does it have any connection to custom frames becoming buggy when loaded from a save file? What other functions are affected by this bug?

 

[Tasyen]

No connection to Frames, as far as I see that. When one puts xpcall over such, it reads "luahelper.lua:38: bad argument #2 to 'unpack' (number expected, got string)"

 

[MyPad]

Interesting stuff indeed.
Hope this can be stickied somewhere for 1.31 Lua mappers to take note of.

 

[Dr Super Good]

This is why it is recommended to keep your Warcraft III up-to-date for general use.

 

[Aniki]

If 'GetFourCC' uses 'string.pack', why not use 'string.unpack' for 'FourCC'? It's what blizzard's version does:

-- Helper function that enables the following syntax: ARCHMAGE = FourCC('Hamg')
function FourCC(id)
    return string.unpack(">I4", id)
end

Anyway... 'GetFourCC' seems like a bad name to me. Maybe could adapt 'I2S' and 'S2I' convention, say: 'FourCcToStr' and 'StrToFourCc' or something?

 

[Dr Super Good]

If 'GetFourCC' uses 'string.pack', why not use 'string.unpack' for 'FourCC'? It's what blizzard's version does:

I think FourCC might use a native implementation that unpacks them faster. String pack/unpack have to deal with multi-byte encoding so will be slower than literally interpreting the value as a sequence of bytes.

 

[Aniki]

String pack/unpack have to deal with multi-byte encoding

Looking in 'lua-src/lstrlib.c/str_pack,str_unpack', I am not seeing anything related to multi-byte encoding. Strings in Lua are "Lua is 8-bit clean: strings can contain any 8-bit value, including embedded zeros ('\0'). Lua is also encoding-agnostic; it makes no assumptions about the contents of a string.".

'string.pack' takes any number of Lua values and serializes them to bytes (according to the format you give it). 'string.unpack' takes some bytes and deserializes them to Lua values (according to the format you give it).

More names for FourCC: 'parse4cc' and 'format4cc' (can you guess which one takes/returns a string =)?).

 

[MyPad]

How about FourCC and CC2Four as complementary functions? CC2Four does the opposite of FourCC and vice versa.

 

[Tasyen]

If 'GetFourCC' uses 'string.pack', why not use 'string.unpack' for 'FourCC'? It's what blizzard's version does:

Because that unpack is the source of the bug after Save&Load. Although unpack seems to be faster and returns 2 values.

Performance Test

TimerStart(CreateTimer(), 0, false, function()
function FourCC1(id)
    return 0x1000000 * string.byte(id:sub(1,1)) +
             0x10000 * string.byte(id:sub(2,2)) +
               0x100 * string.byte(id:sub(3,3)) +
                       string.byte(id:sub(4,4))
end
function FourCC2(id)
    return string.unpack(">I4", id)
end
oldTime = os.clock()
for index = 1, 1000000 do
    FourCC1('hfoo')
end
print(os.clock()-oldTime)


oldTime = os.clock()
for index = 1, 1000000 do
    FourCC2('hfoo')
end
print(os.clock()-oldTime)
end)

Again this is not happening in uptodate, only in 1.31.

 

[Aniki]

This is almost as bizarre as that bug that prevented save games from loading after writing to index 8191 of an array in older patches... save/loading is hard, I guess.

Here's a "goofy" way to stick to 'string.unpack' but not fail after a load:

local strUnpack = string.unpack
local function parse4cc(x)
    return strUnpack('>I4', x)
end

do
    local function patch_1_31_fixup()
        local byte = string.byte
        local sub = string.sub

        parse4cc = function(x)
            return (byte(sub(x, 1,1)) << 24)
                | (byte(sub(x, 2,2)) << 16)
                | (byte(sub(x, 3, 3)) << 8)
                | byte(sub(x, 4, 4))
        end
    end

    local t = CreateTrigger()
    TriggerRegisterGameEvent(t, EVENT_GAME_LOADED)
    TriggerAddAction(t, patch_1_31_fixup)
end

 

[Tasyen]

Someone in Discord (NVS) told me that the bug source prevails in 1.32.9 (current Version). And that it affects string.unpack & string.pack. But as said, normal behaviour is save because FourCC does not use them in that version.

 

[Aniki]

It seems that after a load (at least on v1.31) 'string.unpack' becomes an alias for 'table.unpack', and 'string.pack' becomes an alias for 'table.pack'. If this is true then this seems like a typo kind of error to me.

local function callMeAfterLoad()
    -- after a load 'string.unpack' becomes an alias for 'table.unpack'
    local a, b, c = string.unpack({'a', 'b', 'c'})
    writefln('a: %s, b: %s, c: %s', a, b, c)

    -- after a load 'string.pack' becomes and alias for 'table.pack'
    local t = string.pack('foo', 'bar', 'baz')
    writefln('n: %s, (%s, %s, %s)', t.n, t[1], t[2], t[3])
end

local t = CreateTrigger()
TriggerRegisterGameEvent(t, EVENT_GAME_LOADED)
TriggerAddAction(t, callMeAfterLoad)

 

[MyPad]

In that case, a possible fix for this would have to be storing those two functions either as local values, or as entries in a table, only to be restored (reassigned) when the game is reloaded.

do
    local tb = {pack = string.pack, unpack = string.unpack}
    tb.trig   = CreateTrigger()
    TriggerRegisterGameEvent(tb.trig, EVENT_GAME_LOADED)
    TriggerAddAction(tb.trig, function()
        string.pack, string.unpack = tb.pack, tb.unpack
    end)
end

 

[Tasyen]

sadly, storing the functions and restoring them on load seems not to save this functions.

But this NVS from discord mentioned something wierd.
If one nils this functions on saving then they will work after loading.

The problem I have with that is they will not work anymore for the current match if do that.

 

[MyPad]

Maybe a combination of both approaches might work:

1.) Save the functions as local values
2.) Null functions on saving
3.) Restore the functions after a 0-second timer to keep the functionality as before.

 

[Tasyen]

You are right.
I ended up with a code which does not break after loading anymore. On Save-Event save backups, then nil the real ones, restore pack and unpack after a 0s timer. Finally I had to stop the restoring timer in a Load Event.

do
    local real = InitBlizzard
    function InitBlizzard()
        real()

        -- enable the next line as soon blizzard fixed the bug in current version
        --if GetLocalizedString("REFORGED") ~= "REFORGED" then return end

        local tim  = CreateTimer()
        local trig   = CreateTrigger()
        TriggerRegisterGameEvent(trig, EVENT_GAME_SAVE)
        TriggerAddAction(trig, function()
            local backup =  string.pack
            local backup2 =  string.unpack
            string.pack = nil
            string.unpack = nil
            print("saved")
            TimerStart(tim, 0, false, function()
                string.pack = backup
                string.unpack = backup2
            print("backup")
            end)
        end)
       
        local trig2   = CreateTrigger()
        TriggerRegisterGameEvent(trig2, EVENT_GAME_LOADED)
        TriggerAddAction(trig2, function()
            -- prevent restoring the backups when the game is loaded
            PauseTimer(tim)
            print("Loaded")
        end)

        -- demo Code
        TimerStart(CreateTimer(), 1, true, function()
            xpcall(function()
            local id, id2, id3
            id  = 'Hpal'
            id2 = string.unpack(">I4", id)
            id3 = string.pack(">I4", id2)
            print(id, id2, id3)
            end, print)
        end)
    end
end

 

[Halithor]

I'm curious, is this still an issue in version 1.34?