TarCV / botc

Fork of Dusk's botc compiler *OPEN FOR ADOPTION*
Other
1 stars 0 forks source link

DH_ARRAYSET implementation is wrong #16

Closed ghost closed 4 years ago

ghost commented 4 years ago

I've just went through this issue, I have decided to pick everything necessary from Zandronum's source code to test what happens when the function is called like that. Some things to consider: LONG is an alias for int, bool's size is 1 byte, the memset() below can be translated as memset(array, 1, sizeof(int)*32768), although I am not sure what is the value of lHighestVal in Zandronum's code. What I get from this is that memset() doesn't ever set the value that the modder tells it to even though there is one missing information.

#include <iostream>
#include <cstring>

// Maximum number of global arrays that can be defined in a script.
#define MAX_SCRIPT_ARRAYS                       16

// Maximum number of global arrays that can be defined in a script.
#define MAX_SCRIPTARRAY_SIZE            65536

typedef int LONG;

typedef struct
{
        LONG            alScriptArrays[MAX_SCRIPT_ARRAYS][MAX_SCRIPTARRAY_SIZE];
} SCRIPTDATA_t;

// ===========================================================================//

SCRIPTDATA_t    m_ScriptData;
LONG            lVal            = 1; // value to go into array
LONG            lHighestVal     = 32768;        // Array size?
LONG            lArray          = 1; // equivalent to $globalArray1[]

int main (void)
{
        // same as memset(1, 1, 32768);
        memset( m_ScriptData.alScriptArrays[lArray], lVal, lHighestVal * sizeof( LONG ));
        std::cout << std::hex << m_ScriptData.alScriptArrays[lArray][0] << ", " << m_ScriptData.alScriptArrays[lArray][1] << ", " << m_ScriptData.alScriptArrays[lArray][2] << ", ...\n";
        return 0;
}
TarCV commented 4 years ago

Yes, result of the call memset(1, 1, 32768) in HUMANBOT would be globalArray1 elements all set to 0x01010101. And I'm not sure if memset command is actually useful for a modder. But probably there is nothing to do to fix that on botc side. At least other than making the compiler stable enough to be used for building Zandronum wad, and thus allowing to use bot sources instead of compiled bytecode.

ghost commented 4 years ago

Alright, I've written a smaller, more concise and fool proof answer in the tracker since someone wasn't able to spot the issue.

ghost commented 4 years ago

Is a simple for loop sufficient in the VM? The str type will have the same size as an int, right?

TarCV commented 4 years ago

I'm not sure it's possible to put a string value into a global array in bot scripts at all. I can't find any DH_ code that pops a string stack value and puts it into a variable or an array.

ghost commented 4 years ago

In any case, I have submitted a PR. https://bitbucket.org/Torr_Samaho/zandronum-stable/pull-requests/89/bots-dh_arrayset-implementation-is-wrong/diff

TarCV commented 4 years ago

Thank you. May be it'll even fix some weird behavior of the bots.

ghost commented 4 years ago

Are you referring to this? https://zandronum.com/tracker/view.php?id=3774

ghost commented 4 years ago

Commit corrected and accepted. https://bitbucket.org/Torr_Samaho/zandronum-stable/commits/a04a4f94737e5ad3aad6573227e146b76f037d95