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

Dynamically add player HUD to map scenes #179

Closed
aronand opened this issue Apr 26, 2024 · 14 comments
Closed

Dynamically add player HUD to map scenes #179

aronand opened this issue Apr 26, 2024 · 14 comments
Labels
enhancement New feature or request

Comments

@aronand
Copy link
Contributor

aronand commented Apr 26, 2024

As far as I can tell, currently the HUD has to be added to each map scene for it to work/exist. I propose that it is instead instantiated during scene loading (either in CogitoSceneManager or in CogitoScene), or/and made into a singleton as we'll most likely not have or need multiple instances of the HUD for the player.

Going with a singleton would open new possibilities for updating data as well. This could possibly allow us to signal (some) user interface changes directly via the singleton.

Singleton pseudocode

## UserInterface singleton
var ui_scene = preload("res://.../Player_HUD.tscn").instantiate()
## Scene or SceneManager
scene.add_child(UserInterface.ui_scene)
@aronand
Copy link
Contributor Author

aronand commented Apr 26, 2024

Added a quick implementation as a draft PR.

@Phazorknight Phazorknight added the enhancement New feature or request label Apr 30, 2024
@Phazorknight
Copy link
Owner

Haven't looked at the PR yet, but just wanted to mention that it has previously come up to streamline this a bit more in general.
Some ideas where to add the Pause Menu and Player HUD to the Player scene itself, another approach was to spawn or instance all three of these dynamically at runtime and using the CogitoScene settings for stuff like spawn points.
Will heavily depend on complexity vs versatility.

Ok, will look into your PR now.

@aronand
Copy link
Contributor Author

aronand commented May 1, 2024

Yup, the PR is a quick and dirty change to see if it works, and does just what you described with the HUD and pause menus. The singleton preloads both, and then CogitoScene instantiates them and adds them into the scene.

It's a very small change and visually works, however one thing to verify is that the scene management doesn't work any magic behind the scenes, i.e. ensure there ever is only one instance of both UI elements in the scene, as in one of the variations I tested only the initial scene had the HUD, and this didn't pass into the other scenes.

For my own project I ended up doing the following:

class_name UserInterfaceSingleton


static var _instantiated: bool = false
var _ui_scene: PackedScene = preload("res://core/user_interface.tscn")
var _ui_instance: Control = null
var ui_instance: Control:
	get:
		if _ui_instance == null:
			_ui_instance = _ui_scene.instantiate()
		return _ui_instance


func _init() -> void:
	assert(not _instantiated, "Do not instantiate UserInterfaceSingleton!")
	_instantiated = true

The null check is there, because a scene change seems to also free the instance, even if it comes from the singleton. I didn't too heavily investigate this however, it might be possible to simply reparent the UI scenes instead 🤔

And then the scene simply gets the instance from the singleton:

func _ready() -> void:
	var ui: Control = Icarus.UserInterface.ui_instance
	add_child(ui)

The benefit here being that I can access the ui_instance from anywhere in the code, so I can also connect signals to it easily (although I actually haven't yet tested if it's better to connect to it through the singleton, as that will only get freed when exiting the game).

@Phazorknight
Copy link
Owner

Oh, this looks pretty good, shouldn't add too much complexity?
I'm not too much of a fan off is the string-based path reference to the UI scene in the autoload/singleton though. I know they aren't always avoidable in Godot, but they always give me a stomach ache as they break as soon as you restructure your project a bit.

one thing to verify is that the scene management doesn't work any magic

Scene manager pretty much only moves the Player around and loads player data. I made the UI independent of this, so should be fine. 🤷🏼

The null check is there, because a scene change seems to also free the instance

Yeah, the level scene transition works by completely freeing the level scenes. In this case, having the PlayerHUD and PauseMenu dynamically added to the Level scene might make things a bit more complicated (though not impossible).

Just thinking out loud, I probably lean more towards adding the PauseMenu and PlayerHUD as "components" to the Player.tscn.

Pros:

  • Easier to understand for devs new to Cogito.
  • Easier to hook up signals/references between them.
  • Easier to set up new Cogito level scenes as only the player.tscn will need to be added.
  • Cogito Scene Manager keeps a reference to the currently active Player.tscn, so this can be also used to always have a reference to PlayerHUD and PauseMenu.
  • No need for another autoload/singleton

Cons:

  • Probably not the best practice as it counts as coupling
  • Potential layering issues with the Control nodes / UI elements. Though probably avoidable somehow (want to add functionality to hide/show crosshair etc.)

@aronand
Copy link
Contributor Author

aronand commented May 2, 2024

Oh, this looks pretty good, shouldn't add too much complexity?

Yeah, shouldn't add too much complexity. In this case, as long as a map/level is a CogitoScene the UI will get added to it during runtime. As it's done at the scene's _ready() function, a reference to the player can be passed to the UI as well if necessary, though this can be fetched from SceneManager as well if I remember correctly.

I'm not too much of a fan off is the string-based path reference to the UI scene in the autoload/singleton though [...] they break as soon as you restructure your project a bit.

True, true. For some hard coded strings I'm planning on adding them to a configuration file that can be edited through editor tools, and in case of UI I just hope I won't have to alter the structure too much 😄

Just thinking out loud, I probably lean more towards adding the PauseMenu and PlayerHUD as "components" to the Player.tscn.

That's definitely one way to do it and should work. For my own project I try to decouple things like that, so you don't have to access the player to get to the UI.

Quickly thinking about it, the only edge case where that could be an issue is a game that allows switching the controllable character (for an interesting example, see Battlefield 2: Modern Combat), but I guess that would be more of a game implementation detail in the end, as in how do you handle passing the control/updating the player object.

No need for another autoload/singleton

So I kind of ended up doing this in a hacky way because I like namespaces. I only have a single autoload, that acts as namespace for singletons + is config holder (global toggles etc.). This way I can then access every singleton from a single namespace.

extends Node
## A singleton that acts as a namespace for all other singletons.

# NOTE: An issue with this method of doing singletons, is that the autocompletion
# does not seem to function (at least in Godot 4.2.1)
var UserInterface: UserInterfaceSingleton = UserInterfaceSingleton.new()
var ItemRepository: ItemRepositorySingleton = ItemRepositorySingleton.new()
var Utils: UtilsSingleton = UtilsSingleton.new()

#region Paths
# TODO: Fetch the correct folder from a config instead
var items_path: String = "res://game/assets/items/"
#endregion

#region Global toggles
## Controls interactability globally.
var interactions_allowed: bool = true

## Controls whether interaction prompts are generated globally.
var generate_interaction_prompts: bool = true

## Controls whether carried items can be picked or not.
var can_pick_carried_item: bool = true
#endregion

If that were loaded simply as Cogito, you could use it like Cogito.UserInterface etc.

@ac-arcana
Copy link
Collaborator

ac-arcana commented May 2, 2024 via email

@aronand
Copy link
Contributor Author

aronand commented May 2, 2024

If I understood correctly, essentially we'd want to have the ability to plug in your own UI scenes instead of using the built-in ones, right?

@ac-arcana
Copy link
Collaborator

ac-arcana commented May 2, 2024 via email

@aronand
Copy link
Contributor Author

aronand commented May 2, 2024

Thinking about it, at least with the injection route I think some sort of a Cogito project config would be necessary to achieve that (not necessarily a config, could be some central script as well of course). As long as the custom UI implements a Cogito UI interface (so probably just create a base class which the UI script should extend), it should be good to use.

On the other hand, if attached to the player, the user could create a copy of the player scene, modify it, and use that in their own levels. Might cause some headaches if trying to integrate changes from Cogito to own project, depending on how heavily it otherwise stays dependent on Cogito's scripts, or doesn't.

@ac-arcana
Copy link
Collaborator

ac-arcana commented May 2, 2024 via email

@Phazorknight
Copy link
Owner

Sorry for the late reply, had to deal with my main game project and then worked on some other ends of Cogito.

@aronand: some sort of a Cogito project config would be necessary

I was thinking this more and more recently. In Unity there's the Indexer pattern, that can be used to specify a global singleton that works like a ScriptableObject. I'd love to have something similar in Cogito, though we'd probably have to go the plug-in route of creating a Cogito tab at the top bar to get that level of comfort and usability.
While it might be a bit of work, it could offer an opportunity to create a central place for some global Cogito project settings, especially down the line (like a manager for resources like CogitoItems).

For the 1.0 release I'd probably keep it simpler though.

@ac-arcana
Copy link
Collaborator

ac-arcana commented May 23, 2024

What about a resource for Cogito configuration? It could be top level in the cogito directory and we can point it out in the readme. In Unity terms its like a Scriptable Object

I think I would be happy though with an object in the scene that you point to a player scene which already contains a player, hud, and pause menu. Users could just change the scene it is pointed to. This way all the correct player settings are loaded for their project rather than needing to make sure it is set up correctly in every single scene. Perhaps this could be handled in scene loading.

@Phazorknight
Copy link
Owner

So I've just pushed an update that adds the Player HUD and Pause Menu to the Player.tscn in 64c020d

Please take a loo and see if this makes things a bit easier.

@aronand I've opted for this route for now just for simplicity, feel free to keep your PR #180 open for further experimentation with this.

@Phazorknight
Copy link
Owner

Closing this now as it's no longer an issue per se.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants