Inconsistancy In access modifiers in CesiumTileExcluder

[Reag]

Minor issue I noticed while implementing a custom CesiumTileExcluder implementation:

OnEnable() and OnDisable() are marked as 'protected virtual', which implies that developers are invited to override these methods with their own custom implementations. However, the only way to actually access the functionality of the CesiumTileExcluder is via the AddToTileset and RemoveFromTileset internal methods. Thus only if you are in the Cesium namespace these methods are accessible. Thus if you don't want to call the default OnEnable or OnDisable behaviors, the component cannot have functionality.

In my particular case, I wish to Add and Remove the excluder from the tileset under very specific circumstances. This leaves me with a choice of doing some very 'cursed' monobehaviour enable/disable manipulation or building a special asmdef extension. My actual solution ended up looking like this, with the class being inside the cesium namespace:

public class CesiumTileExcluderInjector
{
    private readonly Cesium3DTileset _tileset;
    private readonly CesiumTileExcluder _excluder;

    public CesiumTileExcluderInjector(Cesium3DTileset tileset, CesiumTileExcluder excluder)
    {
        _tileset = tileset;
        _excluder = excluder;
    }

    public void Add()
    {
        _excluder.AddToTileset(_tileset);
    }

    public void Remove()
    {
        _excluder.RemoveFromTileset(_tileset);
    }
}

[EDIT]: I just noticed, but the description of the CesiumTileExcluder seems to imply that you can place it anywhere in the hierarchy above a tileset for it to work, but it will only work if its below or on the GeoReference. Is this intended? If so, it might be a good idea to give a small update to the description.

[kring]

@Reag can you elaborate on why you can't just enable / disable the MonoBehaviour? That's the intended way to turn the excluder on and off. AddToTileset / RemoveFromTileset definitely should not be called directly. For one thing, if you were to call RemoveFromTileset, and then refresh the tileset itself, the raster overlay would reappear. Disabling the component avoids that issue.

I just noticed, but the description of the CesiumTileExcluder seems to imply that you can place it anywhere in the hierarchy above a tileset for it to work, but it will only work if its below or on the GeoReference. Is this intended?

Yes, generally all Cesium objects need to be on or below the game object with the CesiumGeoreference.

[Reag]

The main issue stems from the network architecture of our product. While the server is aware of all the cesium tilesets that could be loaded (One customer has over 200 in the system), the user clients only create the tilesets when the map moves into their bounding box. This means that as the users are manipulating the map, various tilesets can be added or removed via a networkmanager on the top level of the scene. Finally, the CesiumTileExcluder implementation we built has some moderately complex calculations involved every frame (involving the maps state, dimensions, and if the user has enabled particular functionalities), so we opted to have a single instance of the class manage all our tilesets.

If we where to go with the monobehaviour enable/disable approach, I would have to disable then re-enable the monobehaviour every time this list updates. While definitely a technically valid solution, its also 'cursed'. Add to that the OnEnable and OnDisable involves a 'GetComponentInChildren' call at the top of a large hierarchy, I opted to manage the tileset states manually via an override.

[kring]

While definitely a technically valid solution, its also 'cursed'.

I still don't quite follow what's cursed about it.

Add to that the OnEnable and OnDisable involves a 'GetComponentInChildren' call at the top of a large hierarchy

Have you measured this to take a significant amount of time? Compared to the work that Cesium for Unity needs to do when a raster overlay is added or removed, I'd be really surprised if this is significant.