MantisBT - Zandronum |
View Issue Details |
|
ID | Project | Category | View Status | Date Submitted | Last Update |
0003588 | Zandronum | [All Projects] Bug | public | 2018-12-29 08:04 | 2020-04-27 19:33 |
|
Reporter | FConst | |
Assigned To | | |
Priority | low | Severity | minor | Reproducibility | have not tried |
Status | needs testing | Resolution | open | |
Platform | | OS | | OS Version | |
Product Version | 3.0 | |
Target Version | | Fixed in Version | | |
|
Summary | 0003588: Bots DH_ARRAYSET implementation is wrong |
Description | humanbot (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 | |
Tags | No tags attached. |
Relationships | |
Attached Files | humanbot.txt (22,756) 2018-12-29 08:04 https://zandronum.com/tracker/file_download.php?file_id=2452&type=bug |
|
Issue History |
Date Modified | Username | Field | Change |
2018-12-29 08:04 | FConst | New Issue | |
2018-12-29 08:04 | FConst | File Added: humanbot.txt | |
2018-12-29 10:40 | Filystea | Note Added: 0020283 | |
2018-12-29 10:42 | Filystea | Note Edited: 0020283 | bug_revision_view_page.php?bugnote_id=20283#r12335 |
2018-12-29 17:18 | FConst | Note Added: 0020284 | |
2018-12-29 19:07 | Filystea | Note Added: 0020285 | |
2018-12-29 20:46 | FConst | Note Added: 0020286 | |
2018-12-29 21:23 | Filystea | Note Added: 0020287 | |
2020-04-23 18:20 | sosleepy | Note Added: 0021279 | |
2020-04-27 19:33 | Torr Samaho | Note Added: 0021287 | |
2020-04-27 19:33 | Torr Samaho | Status | new => 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? |
|
|
|
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. |
|
|
|
You can blame naming to be not precise.
but there is always documentation for clearing stuff like that up. |
|
|
|
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. |
|
|
|
|