Map Factory Framework refactor

[david-rhodes]

First, some graphs!

Pre optimization (develop)

Post optimization

You will notice that memory no longer grows forever, and is allocated less frequently. Framerates rarely drop below 30fps, where it would often fall below 15fps (on macbook pro) previously. Mobile performance gains are even higher.

This is a major refactor, but I tried to clean up the code as I went. Hopefully the entire system is more readable and modular, now.

Notes:

• I had to fix all the example scenes, so graphics have changed slightly and map setup is a bit different.
• I've added loads of notes for future improvements and documented all hacks accordingly.
• We still need to merge in @brnkhy's POI demo changes, but that PR is not quite working. Should be a simple merge, though.
• This should enable us to more easily create "zoomable" maps.

Guides, tutorials, and documentation all need to be updated accordingly!

[brnkhy]

some notes after a brief look at this, would be great if we can talk before/after scrum @david-rhodes.

• I think I like this new system with AbstractMap, Visualizer&Provider better. We can extend that to support multiple providers in the future, that might be useful.
• I imagine we'll extract _inactiveTiles to an external caching class in the future?
• #if def statements for positioning logic can be tricky. Some devs will have horrible time debugging what's wrong with their builds because they used some relational position based logic in their side etc. Is there any other way to make this pop out more? Like making it an option/checkbox in visualizer or adding a notification there in the ui etc?
• It's destroying all children on tile dispose. We probably can connect that to the caching logic of factories to reuse those game objects. As noted, it might not be easy though.
• IMap feels more like IMapInfo. Just nitpicking.
• factories doesn't keep track of tiles anymore (no reference to tiles, just data events). Any reason for this?
• not about this refactor but we probably can remove Root property from AbstractMap.
• // TODO: move unitytile state registrations to layer visualizers. Not everyone is interested in this data and we should not wait for it here!
  this might be nice

loving it so far, looks quite good!

[brnkhy]

@david-rhodes
I'm trying something like; (notice pools)

private void CreateTerrainHeight(UnityTile tile)
        {
            var parameters = _parameterPool.GetObject();
            parameters.Fs = _fileSource;
            parameters.Id = tile.CanonicalTileId;
            parameters.MapId = _mapId;
            
            tile.HeightDataState = TilePropertyState.Loading;

            var pngRasterTile = _rasterTilePool.GetObject();
            tile.AsyncRequest = pngRasterTile;
            tile.OnRecycled += (u) => { _rasterTilePool.PutObject(pngRasterTile); };

            pngRasterTile.Initialize(parameters, () =>
            {
                // HACK: we need to check state because a cancel could have happened immediately following a response.
                if (pngRasterTile.HasError || pngRasterTile.CurrentState == Tile.State.Canceled)
                {
                    tile.HeightDataState = TilePropertyState.Error;

                    // HACK: handle missing tile from server (404)!
                    CreateFlatMesh(tile);
                    return;
                }

                tile.SetHeightData(pngRasterTile.Data, _heightModifier);
                GenerateTerrainMesh(tile);
                _parameterPool.PutObject(parameters);
            });
        }

posting here for future reference, let's talk about it today

[david-rhodes]

@brnkhy

I imagine we'll extract _inactiveTiles to an external caching class in the future?

YES! Let's do it.

#if def statements for positioning logic can be tricky. Some devs will have horrible time debugging what's wrong with their builds because they used some relational position based logic in their side etc. Is there any other way to make this pop out more? Like making it an option/checkbox in visualizer or adding a notification there in the ui etc?

This is a good catch. Let's figure out a better way to communicate what happens here.

It's destroying all children on tile dispose. We probably can connect that to the caching logic of factories to reuse those game objects. As noted, it might not be easy though.

Horrible, temporary hack. I think each factory should be responsible for its own cleanup? Mesh modifiers and such are the most difficult, though.

factories doesn't keep track of tiles anymore (no reference to tiles, just data events). Any reason for this?

What do we need a reference to tile for? The factory pattern typically does not keep reference to objects it creates, right? In special cases, like terrain stiching for TerrainFactory, a special internal dictionary is used to map mesh data for nearby tiles. But, not all factories need such data, so those are custom implementations.

not about this refactor but we probably can remove Root property from AbstractMap.

Yeah, this is more of a UnityMap reference. That said, I am using it to properly parent tile gameobjects.