Try out TileManager visitor

[duncanspumpkin]

Just trying to simiplify the code we use for accessing tile elements what about the below:

TileManager::visitAll<BuildingElement>(pos, [=](BuildingElement& elBuilding) {
    if (elBuilding.baseZ() != baseZ())
    {
        return;
    }
    elBuilding.setConstructed(isConstructed);
    elBuilding.setUnk5u(newUnk5u);
    elBuilding.setAge(newAge);
    Ui::ViewportManager::invalidate(pos, elBuilding.baseZ() * 4, elBuilding.clearZ() * 4, ZoomLevel::quarter);
});

TileManager::visitFirst<BuildingElement>(pos, [=](BuildingElement& elBuilding) {
    if (elBuilding.baseZ() != baseZ())
    {
        return false;
    }
    elBuilding.setConstructed(isConstructed);
    elBuilding.setUnk5u(newUnk5u);
    elBuilding.setAge(newAge);
    Ui::ViewportManager::invalidate(pos, elBuilding.baseZ() * 4, elBuilding.clearZ() * 4, ZoomLevel::quarter);
    return true;
});

Advantages:

• No need to deal with Tile.
• You only ever have a reference not a pointer.
• Reduces LOC a small amount
• Expresses intent better visitAll, visitFirst

Disadvantages:

• passing parameters into lambdas isn't always great
• You still need to write BuildingElement twice if you want syntax highlighting.
• visitFirst needs the return true/false whilst visitAll does not.
• No sharing of search parameters (baseZ in above). Could be made into a Visitor and a UnaryPredicate search function (this might also solve the no sharing between visitFirst visitAll
• Lambda's sometimes format weird in clangformat.

[IntelOrca]

Technically visitAll can be achieved by just using visitFirst. Is there need for both?
It would also be nice to have a find and findIndex I think.

[duncanspumpkin]

Technically visitAll can be achieved by just using visitFirst. Is there need for both? It would also be nice to have a find and findIndex I think.

Perhaps it should be findFirst and visitAll. I don't think findIndex is too useful we don't normally use index's.

[ZehMatt]

As for additional parameters you could just have variadic template args after the visitor which you can just forward,

TileManager::visitFirst<BuildingElement>(pos, [](BuildingElement& elBuilding, int32_t baseZ) {
    if (elBuilding.baseZ() != baseZ())
    {
        return false;
    }
    elBuilding.setConstructed(isConstructed);
    elBuilding.setUnk5u(newUnk5u);
    elBuilding.setAge(newAge);
    Ui::ViewportManager::invalidate(pos, elBuilding.baseZ() * 4, elBuilding.clearZ() * 4, ZoomLevel::quarter);
    return true;
}, baseZ);

Now it would be a pure function.

[IntelOrca]

@duncanspumpkin how about just find and visit, both returning a condition to say whether to terminate the loop. We could at least start off with that, and see how it goes.

We will also want to have common find methods anyway, like in OpenRCT2, we have get_track_element_at_with_seq etc. etc.

[duncanspumpkin]

@duncanspumpkin how about just find and visit, both returning a condition to say whether to terminate the loop. We could at least start off with that, and see how it goes.

We will also want to have common find methods anyway, like in OpenRCT2, we have get_track_element_at_with_seq etc. etc.

I find the openrct2 ones quite inflexible leading to lots of similarly named functions. I was thinking perhaps we could have some common search functions like an atHeight noGhosts which you could combine together. So it would end up visit<BuildingElement>(loc, atHeight(25) && noGhosts(), [](auto& elB){ doStuff });

[IntelOrca]

Maybe we could have some sort of method chain...

auto el = tile.find<BuildingElement>()
    .withZ(25)
    .noGhosts()
    .where([](auto& b) { return customPredicate; };

[duncanspumpkin]

Maybe we could have some sort of method chain...

auto el = tile.find<BuildingElement>()
    .withZ(25)
    .noGhosts()
    .where([](auto& b) { return customPredicate; };

I'm not sure how that would work in c++.

[IntelOrca]

Only way I can think of doing it, is to have it set optional fields on a struct which is used for the search. But this is likely to not get optimised by the compiler.

[LeftofZen]

Maybe we could have some sort of method chain...

This is called a fluent interface or fluent api. You just have a set of methods perform a single operation and then return the (mutated) object for the next caller.

Eg https://levelup.gitconnected.com/method-chaining-with-fluent-interface-pattern-d01e75d4bb3d

I have experience with this in C# with LINQ, which is a fluent api defined on IEnumerable (which is basically just a list exposed only via iterators). Sadly there is no getting around iterating the entire list every time you make a fluent 'chain' call, and you have to implement it smartly for it to not constantly allocate for the new 'list'. I think if you implement the api on the iterator instead of the container, then you won't need any allocations to implement it.

As for the actual implementation in C++, you'd use ranges as they are more or less the C++ 'equivalent' of IEnumerable from C#.

[duncanspumpkin]

Maybe we could have some sort of method chain...

This is called a fluent interface or fluent api. You just have a set of methods perform a single operation and then return the (mutated) object for the next caller.

Eg https://levelup.gitconnected.com/method-chaining-with-fluent-interface-pattern-d01e75d4bb3d

I have experience with this in C# with LINQ, which is a fluent api defined on IEnumerable (which is basically just a list exposed only via iterators). Sadly there is no getting around iterating the entire list every time you make a fluent 'chain' call, and you have to implement it smartly for it to not constantly allocate for the new 'list'. I think if you implement the api on the iterator instead of the container, then you won't need any allocations to implement it.

As for the actual implementation in C++, you'd use ranges as they are more or less the C++ 'equivalent' of IEnumerable from C#.

Yeah i had a look at cinq which is a c++ linq wrapper but it has to allocate to handle things. Ranges is all c++20 so not sure we could use it on all platforms. But perhaps i should look into how it handles it.

[LeftofZen]

Maybe we could have some sort of method chain...

This is called a fluent interface or fluent api. You just have a set of methods perform a single operation and then return the (mutated) object for the next caller.
Eg https://levelup.gitconnected.com/method-chaining-with-fluent-interface-pattern-d01e75d4bb3d
I have experience with this in C# with LINQ, which is a fluent api defined on IEnumerable (which is basically just a list exposed only via iterators). Sadly there is no getting around iterating the entire list every time you make a fluent 'chain' call, and you have to implement it smartly for it to not constantly allocate for the new 'list'. I think if you implement the api on the iterator instead of the container, then you won't need any allocations to implement it.
As for the actual implementation in C++, you'd use ranges as they are more or less the C++ 'equivalent' of IEnumerable from C#.

Yeah i had a look at cinq which is a c++ linq wrapper but it has to allocate to handle things. Ranges is all c++20 so not sure we could use it on all platforms. But perhaps i should look into how it handles it.

Oh cool, hadn't heard of cinq before but that looks like exactly what you want. But yeah I think LINQ in C# also allocates, its definitely not efficient, every time I use it in production code I have to consider the performance implications of evaluating the full list.