Skip to content
Open
Show file tree
Hide file tree
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
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that created notification methods are correct, but I've noticed that you set AddressPolicy as Single. We can have multiple CSVSpawner components on the scene. It means that we should be send notification either when all components finishes their jobs (you would need system component for that), or you can change this notification bus AddressPolicy to ById to allow each component to send their own notification. Then it would be client call to decide on which entity/component notification they want to connect with.

Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ namespace CsvSpawner
* @brief Called when entity spawning begins.
* @param m_spawnInfo Struct holding information about entities to be spawned.
*/
virtual void OnEntitiesSpawnBegin(CsvSpawnerUtils::SpawnInfo& m_spawnInfo) = 0;
virtual void OnEntitiesSpawnBegin(CsvSpawnerUtils::SpawnInfo m_spawnInfo) = 0;

/**
* @brief Called when entity spawning finishes.
* @param m_spawnInfo Struct holding information about entities to be spawned.
* @param m_statusCode Status code indicating success, failure and warnings of the spawn.
*/
virtual void OnEntitiesSpawnFinished(CsvSpawnerUtils::SpawnInfo& m_spawnInfo, CsvSpawnerUtils::SpawnStatus m_statusCode) = 0;
virtual void OnEntitiesSpawnFinished(CsvSpawnerUtils::SpawnInfo m_spawnInfo, CsvSpawnerUtils::SpawnStatus m_statusCode) = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason for removing those references?


/// EBus Configuration - Allows multiple listeners to handle events.
static constexpr AZ::EBusHandlerPolicy HandlerPolicy = AZ::EBusHandlerPolicy::Multiple;
Expand All @@ -62,12 +62,12 @@ namespace CsvSpawner
OnEntitiesSpawnBegin,
OnEntitiesSpawnFinished);

void OnEntitiesSpawnBegin(CsvSpawnerUtils::SpawnInfo& m_spawnInfo) override
void OnEntitiesSpawnBegin(CsvSpawnerUtils::SpawnInfo m_spawnInfo) override
{
Call(FN_OnEntitiesSpawnBegin, m_spawnInfo);
}

void OnEntitiesSpawnFinished(CsvSpawnerUtils::SpawnInfo& m_spawnInfo, CsvSpawnerUtils::SpawnStatus m_statusCode) override
void OnEntitiesSpawnFinished(CsvSpawnerUtils::SpawnInfo m_spawnInfo, CsvSpawnerUtils::SpawnStatus m_statusCode) override
{
Call(FN_OnEntitiesSpawnFinished, m_spawnInfo, m_statusCode);
}
Expand Down
60 changes: 31 additions & 29 deletions Gems/CsvSpawner/Code/Source/CsvSpawner/CsvSpawnerUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

#include "CsvSpawnerUtils.h"

#include "AzCore/std/smart_ptr/make_shared.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#include "AzCore/std/smart_ptr/make_shared.h"
#include <AzCore/std/smart_ptr/make_shared.h>


#include <AzFramework/Physics/CollisionBus.h>
#include <CsvSpawner/CsvSpawnerInterface.h>

Expand Down Expand Up @@ -223,11 +225,22 @@ namespace CsvSpawner::CsvSpawnerUtils
{
SpawnInfo broadcastSpawnInfo =
SpawnInfo{ entitiesToSpawn, physicsSceneName, parentId }; // Spawn Info used in CsvSpawner EBus notify.
SpawnStatus spawnStatusCode = SpawnStatus::Success; // Spawn Status Code used for CsvSpawner EBus notify - OnEntitiesSpawnFinished.
// SpawnStatus spawnStatusCode = SpawnStatus::Success; // Spawn Status Code used for CsvSpawner EBus notify -
Copy link
Contributor

Choose a reason for hiding this comment

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

I recommend to remove this code if it is commented

// OnEntitiesSpawnFinished.
auto spawnStatusCode = AZStd::make_shared<SpawnStatus>(
SpawnStatus::Success); // Spawn Status Code used for CsvSpawner EBus notify - OnEntitiesSpawnFinished.

// Call CsvSpawner EBus notification - Begin
CsvSpawnerNotificationBus::Broadcast(&CsvSpawnerInterface::OnEntitiesSpawnBegin, broadcastSpawnInfo);

// Check if there are no entities to spawn
if (entitiesToSpawn.empty())
{
*spawnStatusCode |= SpawnStatus::Fail;
CsvSpawnerNotificationBus::Broadcast(&CsvSpawnerInterface::OnEntitiesSpawnFinished, broadcastSpawnInfo, *spawnStatusCode);
return {};
}

auto sceneInterface = AZ::Interface<AzPhysics::SceneInterface>::Get();
AZ_Assert(sceneInterface, "Unable to get physics scene interface");
const auto sceneHandle = sceneInterface->GetSceneHandle(physicsSceneName);
Expand All @@ -236,6 +249,8 @@ namespace CsvSpawner::CsvSpawnerUtils
auto spawner = AZ::Interface<AzFramework::SpawnableEntitiesDefinition>::Get();
AZ_Assert(spawner, "Unable to get spawnable entities definition");

auto pendingSpawns = AZStd::make_shared<AZStd::atomic_int>(static_cast<int>(entitiesToSpawn.size()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Inside for loop through entitiesToSpawn you have some conditions which cause iteration skip without calling spawning interface. This causes lack of decremention of pendingSpawns what leads to situation where condition in line 349 is never met if as least one entity failed to spawn.


// get parent transform
AZ::Transform parentTransform = AZ::Transform::CreateIdentity();
if (parentId.IsValid())
Expand All @@ -251,18 +266,14 @@ 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))
{
AZ_Error("CsvSpawner", false, "SpawnableAssetConfiguration %s not found", entityConfig.m_name.c_str());

// Add notify code status
spawnStatusCode |= SpawnStatus::Warning;
*spawnStatusCode |= SpawnStatus::Warning;
continue;
}

Expand Down Expand Up @@ -293,7 +304,7 @@ namespace CsvSpawner::CsvSpawnerUtils
else
{
// Add notify code status
spawnStatusCode |= SpawnStatus::Warning;
*spawnStatusCode |= SpawnStatus::Warning;

continue; // Skip this entity if we can't find a valid position and
// place on terrain is enabled.
Expand All @@ -309,7 +320,7 @@ namespace CsvSpawner::CsvSpawnerUtils
if (view.empty())
{
// Add notify code status
spawnStatusCode |= SpawnStatus::Warning | SpawnStatus::Stopped;
*spawnStatusCode |= SpawnStatus::Warning | SpawnStatus::Stopped;

return;
}
Expand All @@ -320,40 +331,31 @@ namespace CsvSpawner::CsvSpawnerUtils
transformInterface->SetWorldTM(transform);
};
optionalArgs.m_completionCallback =
[parentId, &spawnStatusCode, &broadcastSpawnInfo, totalTickets, &completedTickets](
[parentId, spawnStatusCode, pendingSpawns, broadcastSpawnInfo](
[[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.

{
// Add notify code status
spawnStatusCode |= SpawnStatus::Warning | SpawnStatus::Stopped;

return;
*spawnStatusCode |= SpawnStatus::Warning | SpawnStatus::Stopped;
}
else
{
const AZ::Entity* root = *view.begin();
AZ::TransformBus::Event(root->GetId(), &AZ::TransformBus::Events::SetParent, parentId);
Comment on lines +346 to +347
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const AZ::Entity* root = *view.begin();
AZ::TransformBus::Event(root->GetId(), &AZ::TransformBus::Events::SetParent, parentId);
const AZ::Entity* root = *view.begin();
AZ_Assert(root, "Invalid root entity");
AZ::TransformBus::Event(root->GetId(), &AZ::TransformBus::Events::SetParent, parentId);

}
const AZ::Entity* root = *view.begin();
AZ::TransformBus::Event(root->GetId(), &AZ::TransformBus::Events::SetParent, parentId);

completedTickets++;
if (completedTickets == totalTickets)
// Decrement the pending counter
const int remaining = --(*pendingSpawns);
if (remaining == 0)
{
// Call CsvSpawner EBus notification - Finished
// All spawns are finished, now broadcast finished notification
CsvSpawnerNotificationBus::Broadcast(
&CsvSpawnerInterface::OnEntitiesSpawnFinished, broadcastSpawnInfo, spawnStatusCode);
&CsvSpawnerInterface::OnEntitiesSpawnFinished, broadcastSpawnInfo, *spawnStatusCode);
}
};
optionalArgs.m_priority = AzFramework::SpawnablePriority_Lowest;
spawner->SpawnAllEntities(ticket, optionalArgs);
tickets[entityConfig.m_id] = AZStd::move(ticket);

totalTickets++;
}

// 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