@elexis seems not to agree with the solution given that is possible the optimization traverses the points differently, making it to ignore possible better fillings of the area. But I see no notable difference overall from the visual standpoint, which is what I think matters most in this case.
Details
- Reviewers
- None
Diff Detail
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Event Timeline
@elexis seems not to agree with the solution given that is possible the optimization traverses the points differently, making it to ignore possible better fillings of the area. But I see no notable difference overall from the visual standpoint, which is what I think matters most in this case.
This patch consists of multiple changes.
- Unspecified order of traversing. The previous code tried the upper left corner of the given orientation first, whereas the proposed loop goes through a possibly random order.
Also if the area is not something rectangular parallel / orthogonal to 0°, the first big building might not be placed in the corner, resulting in wasted space, lower building density than the currently committed algorithm that was written for that purpose.
n this case the order is specified by the ConvexPolygonPlacer, getPointsInBoundingBox which is the upperleft corner with orientation 0°.
Since the the individual "city districts" are trapezoids and getPointsInBoundingBox returns the leftmost point first, the first point to be checked with your patch will always be one of the 4 corners of the trapezoid. But the corner that it picks is not consistent, while with the rotation in the loop it is.
At least for JB I expect practically equivalent results starting building placement testing with an arbitrary corner first.
- Removal of the +0.5 steps.
Assuming one keeps the for loop and rotation but changes the step from +0.5 to +1, there will be a number of points in the area that won't be tested for building placement.
(red = points missed by +1 steps that +0.5 catches)
Both of these aspects result in a smaller building density than what was planned for. Maybe they are still acceptable, but we really have to test a lot, in particular since it also determines the gameplay whether there are only hundreds of houses built or many elephant stables and fortresses.
I recall in particular the fortress wasn't placed often enough when I hadn't added the extra slow code.
Will have to do many testruns.
JB won't be the only map with this, so it's relevant to judge the Painter by it's task and specifications rather than the one example we have now. For example on the oppidum map it is used with arbitrary areas.
So conclusion is: needs lots of testing to estimate the impact on the "loss of information" on building placement testing.