Add smarter tile rendering, and some miscellaneous changes #5
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
----- Added class names in the HTML for a bunch of tile parts to facilitate manual HTML navigation. ----- Add more view classes for each part of a tile to greatly reduce the size of and logic in `View::Tile`. ----- Rename tabs for tiles/tokens to "All Tiles" and "All Tokens". ----- Logic for rendering curvilinear vs lawson track is cleaner; heavier use of `Engine::Tile#lawson?`. Yellow towns are temporarily rendered as lawson track so that routes to towns can have only track that is used be highlighted. Current curvilinear track segments are edge-to-edge; to render towns on routes properly, we'll need to make curvilinear segments that only go half of an edge-to-edge path. ----- Tile parts that inherit from `View::Part::Base` can set locations where they prefer to be rendered in the function `preferred_render_locations`. `Lib::Tile::REGIONS` defines 25 regions on a tile--the center, each edge, each corner, and "halfway" between the center and each edge and corner. In `preferred_render_locations`, an Array of Hashes is returned. The hashes have `regions` and `transform` keys. `regions` defines where in the 25 regions on the tile this part wants to be rendered; `transform` is a string that will be passed to the SVG `transform` attribute--usually of the form `translate(x y)`. When rendering a new part, a cost is evaluated from `@region_use` the list of regions; whichever location in the `preferred_render_locations` Array has the lowest cost is where the part will be rendered. Then, each of those regions has their "cost" or "use" increased by 1 in `@region_use`. `preferred_render_locations` can also give weighting factors to the regions. This weighting factor affects whether the regions will be used for this part, not how much rendering this part increases the cost in `@region_use`--that is always 1. In this commit, the weighting feature is only used for View::Part::LocationName; location names want to be rendered just above center, or if that is used, just below center, but one track down the middle is worse than two tracks at an angle. Consider B7/Uwajima on the 1889 map. Names can be long, so they include the halfway points on edges 1 and 5 as regions in `preferred_render_locations`, but we'd still prefer to render down there with the 2 track pieces than render up top where there's only 1 track piece, so a weighting factor is applied to reduce the cost of deciding to render below. ----- Even if a part must always be rendered in the same spot (e.g., a segment of lawson-style track), it should implement `preferred_render_locations` so that its regional presence can impact where other parts get rendered.
michaeljb
force-pushed
the
smarter-tile-rendering
branch
from
1bcbe36
to
20eee20
Compare
|
On 20eee20 I switched over the tile part views to use the new triangular grid system, but it was imprecise--rendering the grid over the tile actually came later. More adjustments are needed to get the various parts rendered right where I want them, and to get their I made a couple of aliases for groups of regions, but probably more are warranted. |
tobymao
requested changes
Mar 20, 2020
* update combined_cost, fix its specs - #5 (comment) - #5 (comment) - #5 (comment) * rename parse_tile -> load_from_tile, related changes - #5 (comment) - #5 (comment) - #5 (comment) * simplify tmp code for making towns lawson-style - #5 (comment) * flat_map -> map - #5 (comment) * region_weights must be a hash, can't be array
tobymao
reviewed
Mar 21, 2020
* move move @text definition out of Blocker#load_from_tile - #5 (comment) * refactor and comments for Blocker#should_render? to clarify what it does - #5 (comment) * make View::Part::Cities a subclass of View::Part::Base - #5 (comment) * remove .compact calls from tile and part views; having `nil` in the children array doesn't break rendering - #5 (comment) * call View::Part::CitySlot#on_click with a lambda wrapper in #render instead of making it return a lambda - #5 (comment) * use `if any?` rather than `unless empty?` - #5 (comment) * in View::Part::LocationName, use `!!` to convert value to bool (and configure rubocop to allow this) - #5 (comment) * simplify multi_revenue rendering, it's always a hash - #5 (comment) * drop unneeded var name - #5 (comment) * simpler `select` for offboards in View::Part::Track#render - #5 (comment)
|
Pushing fd215a0, it addresses some more of the comments but I just found it broke something in tile placement, will take a look tomorrow. |
- #5 (comment) also make tiles on tiles page un-clickable
revenue still has its should_render?, so base's render now returns empty string intead of nil if should_render? returns false
"in" - weight to use when determining if this "location" (set of regions) should be used for rendering; could include regions not directly used by this part "out" - weight to use when incrementing cost values in @region_use for the chosen "location"; should noly include regions directly used by this part also, allow for list of regions instead of hash, weight 1.0 is used in that case
|
@tobymao I think I got all previous comments addressed, but may have jumped the gun re-requesting review by a smidge. I'm happy with the look of the tiles on the tile page but a couple on the '89 map are still off (Uwajima name placement and Takamatsu revenue placement), shouldn't take long to fix. |
|
looks good, feel free to merge whenever you're ready |
tobymao
pushed a commit
that referenced
this pull request
May 22, 2021
Merging latest from main repo
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
More detail/explanation in the commit message. For the PR description, I'll let pictures do the talking:
Before:
After:
