Page MenuHomeWildfire Games

BoolArray array wrapper.
Needs ReviewPublic

Authored by nani on Sep 22 2018, 2:06 AM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

Array wrapper that uses uint16 array to store individual bits.

Has two extra methods useful for TileClass: https://code.wildfiregames.com/D1631

Test Plan

Use in Tileclass.js

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

nani created this revision.Sep 22 2018, 2:06 AM
nani created this object with edit policy "nani".
nani changed the visibility from "Public (No Login Required)" to "nani".Sep 22 2018, 2:13 AM
nani changed the edit policy from "nani" to "All Users".
nani edited the summary of this revision. (Show Details)Sep 22 2018, 5:28 PM
nani changed the visibility from "nani" to "Public (No Login Required)".Sep 22 2018, 5:54 PM

Does our spidermonkey support Uint8Array/Uint32Array? It'd be good to compare them by performance/memory usage.

binaries/data/mods/public/maps/random/rmgen/BoolArray.js
2

A comment starts with a capital letter and a space before.

5

Math.ceil(size / 16) => (size + 15) >> 4.

Also some of your lines have semicolons, some don't. Use the same style for whole file. I assume we usually use semicolon always.

11

size / 16 => size & 0xF. The same below.

25

Follow the code convention for naming.

smiley added a subscriber: smiley.Oct 28 2018, 10:45 AM

Why not move this to globalscript/? There could be use cases outside rmgen (triggerscripts comes to mind)

nani updated this revision to Diff 6963.Nov 2 2018, 2:51 PM
nani added a comment.Nov 2 2018, 2:55 PM

Does our spidermonkey support Uint8Array/Uint32Array? It'd be good to compare them by performance/memory usage.

I checked the map gen times for 8 ,16 ,32 in multiple maps and haven't found any notable difference. Haven't checked the memory usage.

Stan added a subscriber: Stan.Nov 2 2018, 6:22 PM
Stan added inline comments.
binaries/data/mods/public/maps/random/rmgen/BoolArray.js
1

uint16_t I think.

40

++i

53

++i

Stan added a comment.Nov 2 2018, 6:23 PM

Your bitwise operations might be more costly than the potential gain.

nani added a comment.Nov 2 2018, 7:41 PM
In D1637#65843, @Stan wrote:

Your bitwise operations might be more costly than the potential gain.

While that might be true the methods isSomeNotZero and isSomeNotOne used in in TileClass.js do improve map generation time (tested 20% improvement average if i remember correctly).

Stan added a comment.Nov 2 2018, 8:13 PM

Sure I'm not saying this is is useless :)
You could cache the results of the bitwise operator in an array maybe. Array access will be faster than any computation.

elexis added a subscriber: elexis.Nov 3 2018, 9:01 AM
In D1637#65603, @smiley wrote:

Why not move this to globalscript?

If the feature proposal is accepted, it should be moved over there.

Does our spidermonkey support Uint8Array/Uint32Array?

Sure, but the problem is that you would have to go through the JS Interface for every read or write access if we use that.

It'd be good to compare them by performance/memory usage.

We need a dedicated test to check the performance gain. The trade-off between the advantages and disadvantages of code complexity and performance must be sufficient.
It could be a file that when executed, runs the same operation with both data structures and measures the time passed.

binaries/data/mods/public/maps/random/rmgen/BoolArray.js
53

(that's the coding convention we use, postfix incrementor only when the return value is read from)

57

(missing semicolon, since L48 is an assignment,)