Page MenuHomeWildfire Games

TileClass optimization.
Needs ReviewPublic

Authored by nani on Sep 22 2018, 1:14 AM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Summary

Optimized TileClass noticing that there were some unnecessary calculations.
All scripts in rmgen only use it to know if a tile is inside or not inside a radius around some tile, making it unnecessary the counting all the tiles inside given radius.

Uses BoolArray to improve up search and memory.
https://code.wildfiregames.com/D1637

This also fixes inconsistency between the >= 27 case and normal algorithm. @elexis

Also worth discussing if RangeOp is to be kept or no for there is still no use case.

Test Plan

Testing various maps.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 7262
Build 11838: arc lint + arc unit

Event Timeline

nani created this revision.Sep 22 2018, 1:14 AM
nani created this object with visibility "nani".
nani edited the summary of this revision. (Show Details)Sep 22 2018, 5:44 PM
nani changed the visibility from "nani" to "Public (No Login Required)".Sep 22 2018, 5:54 PM

Checked the performance benefit of the proposed early-return in TileClass: P142

wraitii added a reviewer: Restricted Owners Package.Apr 22 2019, 9:36 AM
smiley added a subscriber: smiley.Apr 22 2019, 10:19 AM
This comment was removed by smiley.
smiley added inline comments.Apr 22 2019, 10:26 AM
binaries/data/mods/public/maps/random/rmgen/TileClass.js
41

Is this early return necessary? Not early returning maybe useful for a lot of things.
(Performance impact is minimum, and there is still significant room for improvement here. I did mention one way IIRC.)

smiley added inline comments.Apr 22 2019, 10:55 AM
binaries/data/mods/public/maps/random/rmgen/TileClass.js
41

(The conversation in the chat, not cpp stuff.)

Stan added a subscriber: Stan.Apr 22 2019, 11:01 AM
Stan added inline comments.
binaries/data/mods/public/maps/random/rmgen/TileClass.js
15

JSDoc.

21

JSDoc.

65

Add the final newline :)

nani marked 3 inline comments as done.Apr 23 2019, 3:01 AM
nani added inline comments.
binaries/data/mods/public/maps/random/rmgen/TileClass.js
41

Only needs to checks if there is at least one tile that matches so no need to test for more -> early return

nani updated this revision to Diff 7816.Apr 23 2019, 3:04 AM
Owners added a subscriber: Restricted Owners Package.Apr 23 2019, 3:04 AM
Harbormaster completed remote builds in B7259: Diff 7816.
smiley added inline comments.Apr 23 2019, 7:21 AM
binaries/data/mods/public/maps/random/rmgen/TileClass.js
41

For the current constraints, sure. But if someone wants to actually count the number of points, the function and it’s name is wrong.

Stan added inline comments.Apr 23 2019, 8:17 AM
binaries/data/mods/public/maps/random/rmgen/TileClass.js
26

{Object} -> {{"x":Number, "y":Number}} Unless it's a vector 2D in which case {Vector2D} Same where relevant.

nani updated this revision to Diff 7820.Apr 23 2019, 1:03 PM
Harbormaster completed remote builds in B7262: Diff 7820.