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

RFC: use room.name/node.name as global_id if not set #553

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

bolinfest
Copy link
Contributor

This is for discussion, not submission

Coming up with a globally unique id for every ESCItem can be challenging.
In practice, many non-inventory items are restricted to a single room.
This commit explores the idea of deriving the ESCItem.global_id from
the global_id of the containing ESCRoom and the name of the Godot
node for the ESCItem (which is often unique within the ESCRoom as a
matter of good practice, anyway).

The ultimate goal (not demonstrated in this RFC) would be to support
using the "short name" in ESC scripts, for convenience.

@dploeger
Copy link
Collaborator

I'd say, that if an item is to be interacted with, I'd make sure to set a static global ID and if not, I'd just not set a global id at all. I wouldn't rely on a dynamically generated id, but maybe that's just me.

@StraToN
Copy link
Collaborator

StraToN commented Mar 24, 2022

I see a little caveat: node names are not mandatorily unique in the scene tree. They are only unique in their own sub-tree. You can have this case in Godot:

- root
   - node1
      - ESCItem
   - node2
      - ESCItem

@BHSDuncan
Copy link
Collaborator

I'm on the fence, but in the future I'd likely side with what @bolinfest is saying since if you have a lot of rooms/scenes then you have to remember to prefix global IDs or otherwise make them unique. It would be handy to just be able to have a default global ID for an ESCItem and have its actual global ID built off, say, the node path, with the option to change the default global ID if multiple such items are required to be siblings.

@dploeger
Copy link
Collaborator

I'd like to mark this, so we talk about it in the next board meeting, shall we?

@dploeger dploeger added the board-meeting-discussion Up for discussion in the next board meeting label Mar 28, 2022
@bolinfest
Copy link
Contributor Author

#558 is arguably a more comprehensive RFC, and one that is more meaningful to demo if you want to pull down the branch locally.

@balloonpopper
Copy link
Collaborator

@bolinfest I'm all for anything that speeds up room/object creation and improves the user experience. I have some concerns about how this would play out in real world usage (what happens if you use ctrl-d to duplicate a door object and forget to rename it for example) so I think any implementation might need :

  1. to show an error icon in the scene tree if the auto generated name (based on the current nodes in the tree) creates a clash. This would require a check running in the plugin to validate nodes as they're created/edited

  2. to ensure naming consistency somehow - if I create a door right now then create another door, as a user I wouldn't want any behaviors I'd associated with the first door to now associate with the second if the auto-generated IDs regenerated and assigned in the reverse order to the original order.

So I'm not against the theory, just not sure how it would quite work out in practice.

@enekonieto
Copy link
Contributor

enekonieto commented Mar 9, 2023

what happens if you use ctrl-d to duplicate a door object and forget to rename it for example

Perhaps on duplication and automatic rename should be done? For example, duplicating door creates door02, door_copy or something like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
board-meeting-discussion Up for discussion in the next board meeting discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants