Skip to content

Expose prefab instantiation to Lua#816

Open
Gopmyc wants to merge 8 commits into
Overload-Technologies:mainfrom
Gopmyc:808
Open

Expose prefab instantiation to Lua#816
Gopmyc wants to merge 8 commits into
Overload-Technologies:mainfrom
Gopmyc:808

Conversation

@Gopmyc
Copy link
Copy Markdown
Contributor

@Gopmyc Gopmyc commented May 18, 2026

Description

Adds Scene:InstantiatePrefab(...) to Lua with optional local transform and parent.

Related Issue(s)

Fixes #808

Review Guidance

Focus on Lua overloads, prefab path resolution, and prefab source normalization.

Screenshots/GIFs

N/A

AI Usage Disclosure

Refactored code / Debugging

Checklist

  • My code follows the project's code style guidelines
  • When applicable, I have commented my code, particularly in hard-to-understand areas
  • When applicable, I have updated the documentation accordingly
  • My changes don't generate new warnings or errors
  • I have reviewed and take responsibility for all code in this PR (including any AI-assisted contributions)

Copy link
Copy Markdown
Member

@adriengivry adriengivry left a comment

Choose a reason for hiding this comment

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

Instantiate prefab should be a method in OvCore::SceneSystem::Scene, and Lua should expose it. Implementation should not live in Lua bindings.

Comment thread Sources/OvCore/src/OvCore/Scripting/Lua/LuaScriptEngine.cpp
Comment thread Sources/OvCore/src/OvCore/Scripting/Lua/Bindings/LuaGlobalBindings.cpp Outdated
Comment thread Sources/OvCore/src/OvCore/Scripting/Lua/Bindings/LuaGlobalBindings.cpp Outdated
Comment thread Resources/Engine/Lua/Scene/Scene.lua Outdated
@Gopmyc
Copy link
Copy Markdown
Contributor Author

Gopmyc commented May 19, 2026

Instantiate prefab should be a method in OvCore::SceneSystem::Scene, and Lua should expose it. Implementation should not live in Lua bindings.

While reviewing the runtime path, I found that prefab actors spawned during play mode could receive lifecycle callbacks before deserialization was complete. I fixed this by deferring startup until the instantiated hierarchy has been fully deserialized

@Gopmyc Gopmyc requested a review from adriengivry May 19, 2026 04:12
Comment thread Sources/OvCore/src/OvCore/Scripting/Lua/LuaScriptEngine.cpp Outdated
Comment thread Sources/OvCore/include/OvCore/SceneSystem/PrefabRef.h Outdated
Comment thread Sources/OvCore/include/OvCore/SceneSystem/Scene.h Outdated
Comment thread Sources/OvCore/include/OvCore/SceneSystem/Scene.h Outdated
Comment thread Sources/OvCore/src/OvCore/ECS/Components/Behaviour.cpp Outdated
Comment thread Sources/OvCore/src/OvCore/ECS/Components/Behaviour.cpp Outdated
Comment thread Sources/OvCore/src/OvCore/Scripting/Lua/Bindings/LuaGlobalBindings.cpp Outdated
Comment thread Sources/OvCore/src/OvCore/Scripting/Lua/LuaScript.cpp Outdated
Comment thread Sources/OvCore/src/OvCore/Scripting/Lua/LuaScript.cpp Outdated
@Gopmyc Gopmyc requested a review from adriengivry May 19, 2026 22:40
Comment thread Sources/OvCore/include/OvCore/SceneSystem/Scene.h Outdated
Comment thread Sources/OvCore/src/OvCore/SceneSystem/Scene.cpp Outdated
@Gopmyc Gopmyc requested a review from adriengivry May 20, 2026 16:46
Comment on lines +205 to +206
const auto actors = m_batchCreatedActors;
m_batchCreatedActors.clear();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why would we copy m_batchCreatedActors here?

Instead:

const auto& actors = m_batchCreatedActors; // Or you could use m_batchCreatedActors directly in the for loops

// All the for loops go here

m_batchActorCreation = false
m_batchCreatedActors.clear();


OvCore::ECS::Actor* OvCore::SceneSystem::Scene::InstantiatePrefab(const std::string& p_prefabPath)
{
return InstantiatePrefab(p_prefabPath, OvTools::Utils::OptRef<ECS::Actor>{});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use std::nullopt instead of OvTools::Utils::OptRef<ECS::Actor>{}


OvCore::ECS::Actor* OvCore::SceneSystem::Scene::InstantiatePrefab(const std::string& p_prefabPath, ECS::Actor& p_parent)
{
return InstantiatePrefab(p_prefabPath, OvTools::Utils::OptRef<ECS::Actor>{ p_parent });
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can use p_parent implicitly directly instead of OvTools::Utils::OptRef<ECS::Actor>{ p_parent }

const bool shouldBatchActorCreation = m_isPlaying;
if (shouldBatchActorCreation)
{
BeginBatchActorCreation();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You should always call BeginBatchActorCreation.
How it is implemented internally shouldn't matter to InstantiatePrefab.
EndBatchActorCreation should decide which methods to call based on the state of m_isPlaying when it's called.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Expose Prefab instantiation to the Lua API

2 participants