MantisBT - Zandronum
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0003588Zandronum[All Projects] Bugpublic2018-12-29 08:042020-04-27 19:33
ReporterFConst 
Assigned To 
PrioritylowSeverityminorReproducibilityhave not tried
Statusneeds testingResolutionopen 
PlatformOSOS Version
Product Version3.0 
Target VersionFixed in Version 
Summary0003588: Bots DH_ARRAYSET implementation is wrong
Descriptionhumanbot (and probably other bots too) call it with `memset(1, 1, 32768)` (I call it memset in my decompiler, because memset is used under the hood in the implementation) meaning "set every item of the global array 1 to 1". And that globalArray1 is used as an array of booleans in the bot code, so probably the original code was `memset(1, true, 32768)`.
You can check full decompiled code of humanbot in the attached file.

But currently Zandronum implements DH_ARRAYSET using memset:
```
memset( m_ScriptData.alScriptArrays[lArray], lVal, lHighestVal * sizeof( LONG ));
```
As I understand it, it means set every byte of global array lArray to lVal up to byte lHighestVal*sizeof(long). Thus in the code above each item of globalArray1 would be set to 0x01010101.
Although this issue should not affect current bots I think bots VM still be fixed as we have (somewhat broken) Dusk's compiler and I will finish my decompiler soon.
Steps To Reproduce
Additional Information
TagsNo tags attached.
Relationships
Attached Filestxt humanbot.txt (22,756) 2018-12-29 08:04
https://zandronum.com/tracker/file_download.php?file_id=2452&type=bug
Issue History
Date ModifiedUsernameFieldChange
2018-12-29 08:04FConstNew Issue
2018-12-29 08:04FConstFile Added: humanbot.txt
2018-12-29 10:40FilysteaNote Added: 0020283
2018-12-29 10:42FilysteaNote Edited: 0020283bug_revision_view_page.php?bugnote_id=20283#r12335
2018-12-29 17:18FConstNote Added: 0020284
2018-12-29 19:07FilysteaNote Added: 0020285
2018-12-29 20:46FConstNote Added: 0020286
2018-12-29 21:23FilysteaNote Added: 0020287
2020-04-23 18:20sosleepyNote Added: 0021279
2020-04-27 19:33Torr SamahoNote Added: 0021287
2020-04-27 19:33Torr SamahoStatusnew => needs testing

Notes
(0020283)
Filystea   
2018-12-29 10:40   
(edited on: 2018-12-29 10:42)
use man memset if you do not know what it does. It's one of more commonly used C functions.

I read this ( your ticket ))few times and i don't see where is the problem here.
(0020284)
FConst   
2018-12-29 17:18   
Sorry, probably I worded this ticket bad. Basically, I believe intended result of `DH_ARRAYSET(1, 1, 32768)` is an array initialised with ones.
But in Zandronum bot VM arrays are implemented as LONGs, so after applying memset array items will be set to 0x01010101 like inhttps://ideone.com/FeBJgC [^]

Should I make a PR replacing memset with a for loop in DH_ARRAYSET implementation?
(0020285)
Filystea   
2018-12-29 19:07   
Duno how is this related.

But if I understand you correctly...
Imho if it's array of bools and you want all to be true. Than it does not matter. In the end you get something different from zero. In C world it's something that would be treated as true. In C89 we don't even have bool type, int type would be used most cases. Bool appears in C99. When it comes to C++ i have no idea.

If you are worried that bool variable expects only strictly 0 or 1 values of byte in it. I suggest checking standard of C++. Also


Still not entirely sure i get the problem here.
(0020286)
FConst   
2018-12-29 20:46   
Yes, it works for array of bools, but it will produce unexpected result for array of ints, like:
var int $globalArray1[];
var int $index;
onenter {
  DH_ARRAYSET(1, 10, 10); // modder might expect that this will init the array with 10s, but it actually will init the array with different values
  $index = 0;
}
event "..." {
  --$globalArray1[$index];
  if ($globalArray1[$index] == 0) {
    ++$index;
    // do something once particular event is fired 10 times for current index
  }
}

I think DH_ARRAYSET should be implemented as a fast way to initialise integer (or boolean) arrays. And I don't think byte level memset should be exposed to bot scripts.
(0020287)
Filystea   
2018-12-29 21:23   
You can blame naming to be not precise.

but there is always documentation for clearing stuff like that up.
(0021279)
sosleepy   
2020-04-23 18:20   
The issue is that memset() (not the C function, but botscript's function) doesn't set the value it is told to, which can be verified with the code I posted here
<https://github.com/TarCV/botc/issues/16> and here <https://ideone.com/70zkQT>.
memset(1, 1, 32768) (not the C function, but botscript's function) doesn't even fill it with 1s (or 0x1 if you prefer), but 0x1010101 when it should have to fill it with 1s (or 0x1 if you prefer), which makes this a bug.
(0021287)
Torr Samaho   
2020-04-27 19:33   
I added your patch.