Change picking to stop using indices (#1364)

Change `PickResult` to just have a `weak_ptr` to the thing that was picked instead of an index used to retrieve it from the level.
As part of this it made sense to change visibility to be a direct call instead of flowing events up and down.
Also fixed an issue with the auto-hider where it was applying auto-hide even if it was disabled.
Closes #1269

trview.app.tests/ApplicationTests.cpp

@@ -484,25 +484,6 @@ TEST(Application, ResetLayout)
     application->process_message(WM_COMMAND, MAKEWPARAM(ID_WINDOWS_RESET_LAYOUT, 0), 0);
 }
 
-TEST(Application, ViewerRoomVisibilityCaptured)
-{
-    auto [viewer_ptr, viewer] = create_mock<MockViewer>();
-    auto [level_ptr, level] = create_mock<trview::mocks::MockLevel>();
-    EXPECT_CALL(level, set_room_visibility(0, true)).Times(1);
-
-    ILevel::Source level_source = [&](auto&&...) { return std::move(level_ptr); };
-
-    auto application = register_test_module()
-        .with_viewer(std::move(viewer_ptr))
-        .with_level_source(level_source)
-        .build();
-
-    application->open("", ILevel::OpenMode::Full);
-
-    auto room = mock_shared<MockRoom>();
-    viewer.on_room_visibility(room, true);
-}
-
 TEST(Application, LevelLoadedOnReload)
 {
     auto [file_menu_ptr, file_menu] = create_mock<MockFileMenu>();

trview.app.tests/Elements/ItemTests.cpp

@@ -135,3 +135,14 @@ TEST(Item, MutantEggContentsFlags)
     ASSERT_EQ(22, mutant_egg_contents(8));
     ASSERT_EQ(20, mutant_egg_contents(1851));
 }
+
+TEST(Item, VisibilityRaisesOnChanged)
+{
+    auto item = register_test_module().build();
+
+    bool raised = false;
+    auto token = item->on_changed += [&]() { raised = true; };
+    item->set_visible(false);
+
+    ASSERT_EQ(raised, true);
+}

trview.app.tests/Elements/LevelTests.cpp

@@ -434,27 +434,6 @@ TEST(Level, ItemsRenderedWhenEnabled)
     level->render(camera, false);
 }
 
-TEST(Level, SetRoomVisibilty)
-{
-    auto [mock_level_ptr, mock_level] = create_mock<trlevel::mocks::MockLevel>();
-    EXPECT_CALL(mock_level, num_rooms()).WillRepeatedly(Return(1));
-
-    auto room = mock_shared<MockRoom>();
-    EXPECT_CALL(*room, set_visible(false)).Times(1);
-
-    auto level = register_test_module()
-        .with_level(std::move(mock_level_ptr))
-        .with_room_source([&](auto&&...) { return room; })
-        .build();
-
-    bool level_changed_raised = false;
-    auto token = level->on_level_changed += [&]() { level_changed_raised = true; };
-
-    level->set_room_visibility(0, false);
-
-    ASSERT_TRUE(level_changed_raised);
-}
-
 TEST(Level, RoomNotRenderedWhenNotVisible)
 {
     auto [mock_level_ptr, mock_level] = create_mock<trlevel::mocks::MockLevel>();

trview.app.tests/Elements/LightTests.cpp

@@ -299,3 +299,14 @@ TEST(Light, FogBulbTR5)
     ASSERT_EQ(light.radius(), 2.0f);
     ASSERT_EQ(light.type(), LightType::FogBulb);
 }
+
+TEST(Light, VisibilityRaisesOnChanged)
+{
+    auto light = register_test_module().build();
+
+    bool raised = false;
+    auto token = light.on_changed += [&]() { raised = true; };
+    light.set_visible(false);
+
+    ASSERT_EQ(raised, true);
+}

trview.app.tests/Elements/RoomTests.cpp

@@ -356,15 +356,15 @@ TEST(Room, PickTestsEntities)
     auto room = register_test_module().build();
     auto entity = mock_shared<MockItem>();
     ON_CALL(*entity, visible).WillByDefault(Return(true));
-    EXPECT_CALL(*entity, pick).Times(1).WillOnce(Return(PickResult{ true, 0, {}, {}, PickResult::Type::Entity, 10 }));
+    EXPECT_CALL(*entity, pick).Times(1).WillOnce(Return(PickResult{ .hit = true, .distance = 0, .position = {}, .centroid = {}, .type = PickResult::Type::Entity, .item = entity }));
     room->add_entity(entity);
 
     auto results = room->pick(Vector3(0, 0, -2), Vector3(0, 0, 1), PickFilter::Entities);
     ASSERT_EQ(results.size(), 1);
     auto result = results.front();
     ASSERT_EQ(result.hit, true);
     ASSERT_EQ(result.type, PickResult::Type::Entity);
-    ASSERT_EQ(result.index, 10);
+    ASSERT_EQ(result.item.lock(), entity);
 }
 
 /// <summary>
@@ -378,15 +378,15 @@ TEST(Room, PickTestsTriggers)
     auto room = register_test_module().build();
     auto trigger = mock_shared<MockTrigger>();
     ON_CALL(*trigger, visible).WillByDefault(Return(true));
-    EXPECT_CALL(*trigger, pick).Times(1).WillOnce(Return(PickResult{ true, 0, {}, {}, PickResult::Type::Trigger, 10 }));
+    EXPECT_CALL(*trigger, pick).Times(1).WillOnce(Return(PickResult{ .hit = true, .distance = 0, .position = {}, .centroid = {}, .type = PickResult::Type::Trigger, .trigger = trigger }));
     room->add_trigger(trigger);
 
     auto results = room->pick(Vector3(0, 0, -2), Vector3(0, 0, 1), PickFilter::Triggers);
     ASSERT_EQ(results.size(), 1);
     auto result = results.front();
     ASSERT_EQ(result.hit, true);
     ASSERT_EQ(result.type, PickResult::Type::Trigger);
-    ASSERT_EQ(result.index, 10);
+    ASSERT_EQ(result.trigger.lock(), trigger);
 }
 
 /// <summary>
@@ -400,20 +400,20 @@ TEST(Room, PickChoosesClosest)
     auto room = register_test_module().build();
     auto entity = mock_shared<MockItem>();
     ON_CALL(*entity, visible).WillByDefault(Return(true));
-    EXPECT_CALL(*entity, pick).Times(1).WillOnce(Return(PickResult{ true, 0.5f, {}, {}, PickResult::Type::Entity, 5 }));
+    EXPECT_CALL(*entity, pick).Times(1).WillOnce(Return(PickResult{ .hit = true, .distance = 0.5f, .position = {}, .centroid = {}, .type = PickResult::Type::Entity, .item = entity }));
     room->add_entity(entity);
 
     auto entity2 = mock_shared<MockItem>();
     ON_CALL(*entity2, visible).WillByDefault(Return(true));
-    EXPECT_CALL(*entity2, pick).Times(1).WillOnce(Return(PickResult{ true, 1.0f, {}, {}, PickResult::Type::Entity, 10 }));
+    EXPECT_CALL(*entity2, pick).Times(1).WillOnce(Return(PickResult{ .hit = true, .distance = 1.0f, .position = {}, .centroid = {}, .type = PickResult::Type::Entity, .item = entity2 }));
     room->add_entity(entity2);
 
     auto results = room->pick(Vector3(0, 0, -2), Vector3(0, 0, 1), PickFilter::Entities | PickFilter::Triggers);
     ASSERT_EQ(results.size(), 2);
     auto result = results.front();
     ASSERT_EQ(result.hit, true);
     ASSERT_EQ(result.type, PickResult::Type::Entity);
-    ASSERT_EQ(result.index, 5);
+    ASSERT_EQ(result.item.lock(), entity);
     ASSERT_EQ(result.distance, 0.5f);
 }
 
@@ -428,7 +428,7 @@ TEST(Room, PickChoosesEntityOverTrigger)
     auto room = register_test_module().build();
     auto entity = mock_shared<MockItem>();
     ON_CALL(*entity, visible).WillByDefault(Return(true));
-    EXPECT_CALL(*entity, pick).Times(1).WillOnce(Return(PickResult{ true, 1.0f, {}, {}, PickResult::Type::Entity, 5 }));
+    EXPECT_CALL(*entity, pick).Times(1).WillOnce(Return(PickResult{ .hit = true, .distance = 1.0f, .position = {}, .centroid = {}, .type = PickResult::Type::Entity, .item = entity }));
     room->add_entity(entity);
 
     auto trigger = mock_shared<MockTrigger>();
@@ -440,7 +440,7 @@ TEST(Room, PickChoosesEntityOverTrigger)
     auto result = results.front();
     ASSERT_EQ(result.hit, true);
     ASSERT_EQ(result.type, PickResult::Type::Entity);
-    ASSERT_EQ(result.index, 5);
+    ASSERT_EQ(result.item.lock(), entity);
     ASSERT_EQ(result.distance, 1.0f);
 }
 
@@ -707,7 +707,7 @@ TEST(Room, PickTestsStaticMesh)
     auto static_mesh = mock_shared<MockStaticMesh>();
     ON_CALL(*static_mesh, number).WillByDefault(Return(10));
     ON_CALL(*static_mesh, visible).WillByDefault(Return(true));
-    EXPECT_CALL(*static_mesh, pick).Times(1).WillOnce(Return(PickResult{ true, 0, {}, {}, PickResult::Type::StaticMesh, 10 }));
+    EXPECT_CALL(*static_mesh, pick).Times(1).WillOnce(Return(PickResult{ .hit = true, .distance = 0, .position = {}, .centroid = {}, .type = PickResult::Type::StaticMesh, .static_mesh = static_mesh }));
 
     auto static_mesh_source = [&](auto&&...)
         {
@@ -725,5 +725,18 @@ TEST(Room, PickTestsStaticMesh)
     auto result = results.front();
     ASSERT_EQ(result.hit, true);
     ASSERT_EQ(result.type, PickResult::Type::StaticMesh);
-    ASSERT_EQ(result.index, 10);
+    ASSERT_EQ(result.static_mesh.lock(), static_mesh);
 }
+
+
+TEST(Room, VisibilityRaisesOnChanged)
+{
+    auto room = register_test_module().build();
+
+    bool raised = false;
+    auto token = room->on_changed += [&]() { raised = true; };
+    room->set_visible(false);
+
+    ASSERT_EQ(raised, true);
+}
+

trview.app.tests/Elements/TriggerTests.cpp

@@ -71,3 +71,14 @@ TEST(Trigger, Colour)
     trigger->set_colour(std::nullopt);
     ASSERT_EQ(trigger->colour(), ITrigger::Trigger_Colour);
 }
+
+TEST(Trigger, VisibilityRaisesOnChanged)
+{
+    auto trigger = register_test_module().build();
+
+    bool raised = false;
+    auto token = trigger->on_changed += [&]() { raised = true; };
+    trigger->set_visible(false);
+
+    ASSERT_EQ(raised, true);
+}

trview.app.tests/Lua/Elements/Lua_CameraSinkTests.cpp

@@ -231,11 +231,8 @@ TEST(Lua_CameraSink, SetType)
 
 TEST(Lua_CameraSink, SetVisible)
 {
-    auto level = mock_shared<MockLevel>();
-    EXPECT_CALL(*level, set_camera_sink_visibility(100, true));
-
     auto cs = mock_shared<MockCameraSink>()->with_number(100);
-    EXPECT_CALL(*cs, level).WillRepeatedly(Return(level));
+    EXPECT_CALL(*cs, set_visible(true)).Times(1);
 
     LuaState L;
     lua::create_camera_sink(L, cs);

trview.app.tests/Lua/Elements/Lua_ItemTests.cpp

@@ -259,11 +259,8 @@ TEST(Lua_Item, SetCategories)
 }
 TEST(Lua_Item, SetVisible)
 {
-    auto level = mock_shared<MockLevel>();
-    EXPECT_CALL(*level, set_item_visibility(100, true));
-
     auto item = mock_shared<MockItem>()->with_number(100);
-    EXPECT_CALL(*item, level).WillRepeatedly(Return(level));
+    EXPECT_CALL(*item, set_visible(true)).Times(1);
 
     LuaState L;
     lua::create_item(L, item);

trview.app.tests/Lua/Elements/Lua_LightTests.cpp

@@ -341,18 +341,14 @@ TEST(Lua_Light, Visible)
     ASSERT_EQ(true, lua_toboolean(L, -1));
 }
 
-
 TEST(Lua_Light, SetVisible)
 {
-    auto level = mock_shared<MockLevel>();
-    EXPECT_CALL(*level, set_light_visibility(100, true));
-
     auto light = mock_shared<MockLight>()->with_number(100);
-    EXPECT_CALL(*light, level).WillRepeatedly(Return(level));
+    EXPECT_CALL(*light, set_visible(true)).Times(1);
 
     LuaState L;
     lua::create_light(L, light);
     lua_setglobal(L, "l");
 
     ASSERT_EQ(0, luaL_dostring(L, "l.visible = true"));
-}
+}

trview.app.tests/Lua/Elements/Lua_RoomTests.cpp

@@ -357,11 +357,8 @@ TEST(Lua_Room, Visible)
 
 TEST(Lua_Room, SetVisible)
 {
-    auto level = mock_shared<MockLevel>();
-    EXPECT_CALL(*level, set_room_visibility(100, true));
-
     auto room = mock_shared<MockRoom>()->with_number(100);
-    EXPECT_CALL(*room, level).WillRepeatedly(Return(level));
+    EXPECT_CALL(*room, set_visible(true)).Times(1);
 
     LuaState L;
     lua::create_room(L, room);

trview.app.tests/Lua/Elements/Lua_TriggerTests.cpp

@@ -190,11 +190,8 @@ TEST(Lua_Trigger, Visible)
 
 TEST(Lua_Trigger, SetVisible)
 {
-    auto level = mock_shared<MockLevel>();
-    EXPECT_CALL(*level, set_trigger_visibility(100, true));
-
     auto trigger = mock_shared<MockTrigger>()->with_number(100);
-    EXPECT_CALL(*trigger, level).WillRepeatedly(Return(level));
+    EXPECT_CALL(*trigger, set_visible(true)).Times(1);
 
     LuaState L;
     lua::create_trigger(L, trigger);