Skip to content

[2409] Csv Spawner Notification Bus #109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 48 commits into
base: o3de-2409
Choose a base branch
from
Open

[2409] Csv Spawner Notification Bus #109

Changes from 1 commit
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
6470d8a
init spline subscriber
w-czerski Jan 28, 2025
6ccf97e
clang format
w-czerski Jan 28, 2025
915d029
fix | get spline and convert into vector of vertex
w-czerski Jan 28, 2025
1b9affd
remove auto
w-czerski Jan 28, 2025
57322f8
apply review changes
w-czerski Jan 28, 2025
ee5c565
remove comments
w-czerski Jan 28, 2025
399c2d1
use emplace | format | apply reviews
w-czerski Jan 28, 2025
5b7dd26
change ros2 -> ROS 2
w-czerski Mar 18, 2025
3fc175d
review resolved | topic changed to spline
w-czerski Mar 18, 2025
9cf218a
review resolved | use proper ROS 2 naming
w-czerski Mar 18, 2025
4651355
get ros2frame outside tick
w-czerski Mar 18, 2025
45bd34c
add update frequency
w-czerski Mar 18, 2025
ffa7f34
add readme info | add update frequency
w-czerski Mar 18, 2025
23e9891
add readme info | add update frequency
w-czerski Mar 18, 2025
7049997
Adjust Pointcloud, ImGuiProvider, ImGuizmo code to 2505 version of o3…
michalpelka Jun 30, 2025
c91a03a
Update GeoJSONSpawner to be compatible with O3DE 2505 (#124)
patrykantosz Jun 30, 2025
f70295a
init | add notification bus interface class
w-czerski Jan 28, 2025
5373024
clang format
w-czerski Jan 28, 2025
09e44bf
remove not needed info in bus
w-czerski Jan 28, 2025
ee39e1c
remove bus info for spawn tickets
w-czerski Jan 28, 2025
2fbc807
move spawn utils to public include | spawn info bus struct
w-czerski Jan 29, 2025
002c35b
add notify status code
w-czerski Jan 29, 2025
139bb38
fix typo
w-czerski Jan 29, 2025
318d19f
more context to enum spawn
w-czerski Jan 29, 2025
0c45656
fix format | resolve errors
w-czerski Jan 29, 2025
bb29e40
rename member
w-czerski Jan 29, 2025
5a24a36
add default override implentation | make override optional
w-czerski Jan 29, 2025
54e6f12
add description
w-czerski Jan 29, 2025
22c3628
undo make public utils
w-czerski Mar 18, 2025
51e1941
remove const in func definition
w-czerski Mar 18, 2025
476f2b4
make interface clas final
w-czerski Mar 18, 2025
26cb4bd
init value definition | add spawn stop break points
w-czerski Mar 18, 2025
62358ef
remove const& from struct definition
w-czerski Mar 18, 2025
9e3cf1a
remove final from interface
w-czerski Mar 18, 2025
95b70d1
use constexpr
w-czerski Mar 18, 2025
b30aac2
move struct to utils
w-czerski Mar 18, 2025
75e3739
add lua / script canvas support
w-czerski Mar 18, 2025
61fc9aa
add info to readme
w-czerski Mar 18, 2025
13671a4
fix headers | make interface func pure virtual
w-czerski Mar 18, 2025
2c17d49
fix reflections
w-czerski Mar 18, 2025
896f067
revert 3rdParty changes
w-czerski Jul 2, 2025
687aff9
add counter to track if all entities are spawned
w-czerski Jul 2, 2025
b30dda3
clang format
w-czerski Jul 2, 2025
3694333
Merge branch 'main' into wc/csvspawner_notifications
w-czerski Jul 2, 2025
2e4d01a
remove unused directives
w-czerski Jul 2, 2025
3345918
clang format
w-czerski Jul 2, 2025
48cd8d2
remove dangling references and pointers | using shared ptr for async …
w-czerski Jul 2, 2025
24c4e8d
add spacing
w-czerski Jul 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions Gems/CsvSpawner/Code/Source/CsvSpawner/CsvSpawnerUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,10 @@ namespace CsvSpawner::CsvSpawnerUtils
}
}

// Track how many tickets are spawned and how many have completed
size_t totalTickets = 0;
std::atomic_size_t completedTickets = 0;

for (const auto& entityConfig : entitiesToSpawn)
{
if (!spawnableAssetConfiguration.contains(entityConfig.m_name))
Expand Down Expand Up @@ -317,7 +321,7 @@ namespace CsvSpawner::CsvSpawnerUtils
transformInterface->SetWorldTM(transform);
};
optionalArgs.m_completionCallback =
[parentId, &spawnStatusCode](
[parentId, &spawnStatusCode, &broadcastSpawnInfo, &tickets, totalTickets, &completedTickets](
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears that the four references in question are currently dangling. This arises because local variables are being passed to the completion callback, which is executed after the SpawnEntities function has ended. Once the function finishes, the references to the callback become invalid since the variables they point to no longer exist. It is important to address this issue to ensure the code functions as intended.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reference to tickets is not only dangling but also remains unused, which consequently interferes with building O3DE in the profile configuration.

[[maybe_unused]] AzFramework::EntitySpawnTicket::Id ticketId, AzFramework::SpawnableConstEntityContainerView view)
{
if (view.empty())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code within this if statement exhibits undefined behavior because spawnStatusCode is a dangling reference. This may be the reason why you have not encountered any issues, as the code possibly has not executed as expected. Additionally, the code is missing a notification call to indicate a failure. A similar situation is present in the preinsertionCallback.

Expand All @@ -329,16 +333,28 @@ namespace CsvSpawner::CsvSpawnerUtils
}
const AZ::Entity* root = *view.begin();
AZ::TransformBus::Event(root->GetId(), &AZ::TransformBus::Events::SetParent, parentId);

completedTickets++;
if (completedTickets == totalTickets)
{
// Call CsvSpawner EBus notification - Finished
CsvSpawnerNotificationBus::Broadcast(&CsvSpawnerInterface::OnEntitiesSpawnFinished, broadcastSpawnInfo, spawnStatusCode);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if we assume that completedTickets is not a dangling reference, the current condition will not function as intended. The issue arises because totalTicket is passed as a copy and incremented after the callback is defined. Consequently, completedTickets will always be one greater than totalTickets, resulting in the condition never being met. If you move the totalTickets++ before defining the callback, it would trigger with every callback. Therefore, it is advisable to explore a different approach to address this condition.

};
optionalArgs.m_priority = AzFramework::SpawnablePriority_Lowest;
spawner->SpawnAllEntities(ticket, optionalArgs);
tickets[entityConfig.m_id] = AZStd::move(ticket);

totalTickets++;
}

// Check is success spawn
tickets.empty() ? spawnStatusCode |= SpawnStatus::Fail : spawnStatusCode |= SpawnStatus::Success;
// Call CsvSpawner EBus notification - Finished
CsvSpawnerNotificationBus::Broadcast(&CsvSpawnerInterface::OnEntitiesSpawnFinished, broadcastSpawnInfo, spawnStatusCode);
// If no tickets were created at all (no entities spawned), send finish immediately
if (totalTickets == 0)
{
spawnStatusCode |= SpawnStatus::Fail;
// Call CsvSpawner EBus notification - Finished
CsvSpawnerNotificationBus::Broadcast(&CsvSpawnerInterface::OnEntitiesSpawnFinished, broadcastSpawnInfo, spawnStatusCode);
}

return tickets;
}
Expand Down