TarCV / botc

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

memset() is arrayset() #15

Closed ghost closed 4 years ago

ghost commented 4 years ago

After navigating through Zandronum's source code, I found the following in src/bots.cpp:

                case DH_ARRAYSET:

                        {
                                LONG    lArray;
                                LONG    lVal;
                                LONG    lHighestVal;
//                              LONG    lIdx;

                                lArray = m_ScriptData.alStack[m_ScriptData.lStackPosition - 3];
                                lVal = m_ScriptData.alStack[m_ScriptData.lStackPosition - 2];
                                lHighestVal = m_ScriptData.alStack[m_ScriptData.lStackPosition - 1];

                                if (( lArray < 0 ) || ( lArray >= MAX_SCRIPT_ARRAYS ))
                                        I_Error( "ParseScript: Invalid array index, %d, in command \"arrayset\"!", static_cast<int> (lArray) );
                                if (( lHighestVal < 0 ) || ( lHighestVal >= MAX_SCRIPTARRAY_SIZE ))
                                        I_Error( "ParseScript: Invalid array maximum index, %d, in command \"arrayset\"!", static_cast<int> (lHighestVal) );

                                memset( m_ScriptData.alScriptArrays[lArray], lVal, lHighestVal * sizeof( LONG ));
                                PopStack( );
                                PopStack( );
                                PopStack( );
                        }
                        break;

since DH_ARRAYSET is what we call now memset(), and the error says an error occurred with the command "arrayset", I suggest a renaming of that function, or the creation of an alias for it.

TarCV commented 4 years ago

I really don't like naming it arrayset, because that would imply different implementation like this:

for (size_it i = 0; i < HighestVal; ++i) {
  m_ScriptData.alScriptArrays[lArray][i] = lVal;
}

So I'll leave this issue open, but I'll not accept renaming PR for it. At least not until I hear the word of god (Torr), that both the implementation should be memset and name of the command should be arrayset.

TarCV commented 4 years ago

Now that the fix is merged to zandronum-stable, I'm adding this function definition as arrayset to botc. BTW, do you know if there are official builds for that stable repo?

ghost commented 4 years ago

Yes, here. https://zandronum.com/forum/viewtopic.php?f=55&t=9937&sid=f5cdcfd8304e895f7e72d54ae895b0a2