Event Timeline
Havent really checked for correctness. This was pretty old. Pasted because this was almost lost. Some stuff might be useful.
And I didnt really feel like creating a whole new proposal just for these. Can fix when changing something near.
-function SimpleGroup(objects, avoidSelf = false, tileClass = undefined, centerPosition = undefined)
+function SimpleGroup(objects, avoidSelf = false, tileClass, centerPosition)
Consequentially the = false could be dropped too. But mostly the = undefined is a way to inform the reader that the argument is optional. It's not obvious for the centerPosition and tileclass.
// Vector -> Why? @param
+ minSize || 1, // would have been prettier if these were up there
I feel the hardcoded constants in rmgen-common make more trouble than they save work. If they are removed, the proxies don't save work but only hide the mechanics of placement, making it harder to adjust, growing the libraries bigger without providing something useful. Just endless monkeywork to replace all that.
Any reason for not having centered placers implement the centered placer interface method setCenterPosition?
- return min + (max - min) * (g_MapSettings.Size - minMapSize) / (maxMapSize - minMapSize);
+ return min + (max - min) * (g_Map.getSize() - minMapSize) / (maxMapSize - minMapSize);
g_Map should be and is defined by the map. Where the library refers to it its a defect.
One could consider passing g_MapSettings to the mapinit and read via g_Map if one wants to get rid of g_MapSettings. The g_Map instance should be passed, or it could be called to become the this keyword or similar.
+ if (texture in this.textureNames)
could also be this.textureNames[texture], also maybe textureName, anyway
would be the textureID
is
g_ActorPrefix
Ideally not existing, but inlining it sounds a bit like cheating. The logic itself should become unneeded by redesigning the code flow somehow - otherwise we can keep it.
Looking at the header of maps where all the terrain textures and entity template names are defined, we could perhaps have a constructor for each of them, like Terrain(textureName, actorTemplate = undefined), ActorEntityTemplate(), SimulationEntityTemplate(). Then the actorTemplate function could be removed, the global too and there was only occurrence of "actor|". Maybe. Would have to test if that's better.
tilesToFraction exists only in 5 lines, could be daring and inline.
Wondering if we could silently dump fractionToTiles as well. foo(x) doesnt seem much more readable or useful than x * g_Map.getSize().
Problem is 277 replacements for the gain of 3 lines less. (So you see why it's important to insert new code cleanly rather than cleaning up afterwards)
1.0 -> 1
Didn't check if Object.js change is correct.
Better split patches by logic and start writing them when we can commit them, so we avoid merge conflict hell.