Zandronum Chat @ irc.zandronum.com
#zandronum
Get the latest version: 3.0
Source Code

View Issue Details Jump to Notes ] Issue History ] Print ]
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.
Attached Filestxt file icon humanbot.txt [^] (22,756 bytes) 2018-12-29 08:04 [Show Content]

- Relationships

-  Notes
User avatar (0020283)
Filystea (reporter)
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.

User avatar (0020284)
FConst (reporter)
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?
User avatar (0020285)
Filystea (reporter)
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.
User avatar (0020286)
FConst (reporter)
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.
User avatar (0020287)
Filystea (reporter)
2018-12-29 21:23

You can blame naming to be not precise.

but there is always documentation for clearing stuff like that up.
User avatar (0021279)
sosleepy (reporter)
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.
User avatar (0021287)
Torr Samaho (administrator)
2020-04-27 19:33

I added your patch.

Issue Community Support
Only registered users can voice their support. Click here to register, or here to log in.
Supporters: No one explicitly supports this issue yet.
Opponents: No one explicitly opposes this issue yet.

- 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 View Revisions
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






Questions or other issues? Contact Us.

Links


Copyright © 2000 - 2020 MantisBT Team
Powered by Mantis Bugtracker