Skip to content
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.

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

Fix runtime release crash and add functionality to scene.hpp #13

Merged
merged 7 commits into from
Jun 2, 2024

Conversation

Mangonels
Copy link
Collaborator

Notable changes:

  • level.cpp: In order to complete the functionality cycle of scene packing, added:
    • node::set_owner
    • packer::pack - constructs and returns a PackedScene* from a godot::Node*
    • resource::saver::get - returns ResourceSaver* (we already had loader::get which returns ResourceLoader* here so I'd asume it's not a bad spot)
      I use these on my own project and thought it would be nice if roguelite had this functionality, as a base library of sorts, but
      I'd understand if it were considered out of scope for the game itself, since it doesn't really require it.
  • level.cpp: Renamed scene class to "packed_scene" to better represent what the class contains.
  • level.cpp: Added an overloaded constructor for packed_scene, now accepting a pointer to an existing node for packing into the packed_scene object, while preserving the original constructor that takes a path for ResourceLoader.
  • level.cpp: Added save member function to packed_scene, enabling the packed resource.
  • level.cpp: Removed FileAccess::open check in favour of ResourceLoader::exists, solving a release runtime crash.
  • level.cpp: Removed hacky "std::is_base_of_v" that does set_controller for objects which are Characters, in favour of actually calling that method right after the object is created, decoupling scene.hpp from Character instances.
  • conversions.hpp: Added gd_str_conv, might be useful for compile time conversions. Not actually used anywhere in roguelite, but I tested it on my own project and it works ok.

There's ambiguity between the terms "resource" and "scene" throughout level.cpp's packed_scene class (a scene seems to be considered a resource in godot).
But I think referring to scenes as resources is alright.

I tested the specific additions (such as packing from instanced node and saving packed resource) on my roguelite templated project and the overall functionality and intent for it works good for me.
Of course I also tested this branch of the game in debug from VSC, and in release as windows standalone (where it now runs fine, I believe it wouldn't have run for anyone previously).

I thought of adding some tests to prove all this stuff works, but really they'd just be a useless apendage to the sample roguelite game. Can still do it if required though.

@vorlac
Copy link
Owner

vorlac commented May 13, 2024

i'll take a closer look at why the linux builds are failing for this and the other PR at some point this week

@vorlac vorlac merged commit 660f0a5 into main Jun 2, 2024
2 checks passed
@vorlac vorlac deleted the scene.hpp-improvements branch June 2, 2024 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants