-
-
Notifications
You must be signed in to change notification settings - Fork 97
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
Add static type hints for dictionary members #56
Comments
this would be really nice, yes at the moment for example i really wanted to add a dictionary such that i can add other types not on the list, for example: <String, AudioStream> for a keyed list of sounds i can load from the inspector we have it already with lists |
This would be a very nice feature. I'm doing a VN and would like to replicate the functionality of SpriteFrames in Animated Sprite to help pack all the images for a character into a scene to make it easier to compartmentalize. But using SpriteFrames causes all the images in it to be loaded, even if they aren't used. Which is understandable, but if we packed over 100 images into a character, suddenly it's going to eat up a lot of video memory and likely won't play nice with mobile. All I really need is the path to each image so I can load them when they're needed. So a Dictionary would be a perfect way to replicate the SpriteFrames resource when calling on the sprite with parameters of which sprite resources need to be loaded. But setting up the key and value type for hundreds of images is a chore. So this would help a lot. |
While I don't know/care what the short-hand export syntax would look like In the context of func _get_property_list() -> Array:
return [
{ name = "my_dict", type = TYPE_DICTIONARY,
hint_types = [
{ type = TYPE_STRING }, #key property
{ type = TYPE_INT, hint = PROPERTY_HINT_RANGE, hint_string = "0,10" }, #value property
]
}
] The above somewhat mimics the "Generics" list in some languages, which may also play well when hooking c# in. The editor itself could then do a more recursive-ish rendering of inspectors when it comes to Dictionaries or Arrays. There would be no real depth-limit to the underlying representation. As array stands in 3.3, |
I came here from godotengine/godot#25157 since someone said that that issue continues here. |
This feature isn't planned for 4.0, as we don't intend to add new features to GDScript to ensure it can stabilize for 4.0. |
Is this feature planned for at least 4.x? I'd love to see this feature in 4.1 or 4.2. |
I think it's fair to say that it is desired by people who use static typing inside and outside of the core team. But we'll get to it when we get to it. Unless @vnen wants to pinky promise?.. 🙃 |
Godot 4 has typed dictionaries (I think) and arrays, so I doubt that import hints are required anymore. |
Godot 4.0 only has typed arrays, not typed dictionaries. |
It is not explicitly stated here but a syntax like this would be great (and consistent with the
|
But what happens with nested dictionarys? This syntax works only if dictionary is used like hashmap, allowing only one type as value. Lets say you parse json (that can have various types, nested dictionaries of various types etc) to dictionary, that would require typescript-like interfacing or something, otherwise it seems a bit pointless and it would make more sense to introduce completely new type (hashmap, map or whatever), instead of allowing typing for just this one use case. |
If you have to nest dictionaries, perhaps introducing new types might be a good alternative. This makes things way more readable and easier to test. Compare this (pseudo-code)
to something like this instead:
|
Yeah, nested dictionaries are a whole another beast, but I'd expect them to work the same. I guess we see a reason why some languages opt for "<>" to mark typing, especially if you use square brackets for arrays. My eyes do start looking for the array in that. |
This works but adds little overhead in cases where the data is updated fairly often since you need to turn those dictionaries into class objects (or update them) every time. |
@StatisMike I don't see why you believe it would be any more confusing than statically-typed arrays. You can still use varying-type Arrays and static-type Arrays in the engine with that style of syntax, so why does expanding that feature to a Dictionary cause confusion? I'm possibly off the mark, but introducing |
@StatisMike (& @SquidgySapphic) Also, well, strong typed languages using hash maps have them have strongly typed, too. use hashbrown::HashMap;
fn main() {
// this dictionary is my baby boy and i love him
let mut m = HashMap::<&str, i32>::new();
m.insert("A", 3);
// look at him gooo
dbg!(m.get("A"));
} Allowing Dictionaries to be typed in this way can provide both more type hints for a smoother/safer programming experience, but might drastically improve the performance of Dictionary in necessary cases by removing the need for all the indirection and type checks. Variant Type can perhaps be achieved by |
@StatisMike as it was stated previously, dictionaries are meant to be the exact same thing as a hashmap by design, but the explicit typing is missing. The one "derailment" is that people use this hashmap as a struct. |
I'm adding a simple loot table to my game and an array with nested dictionaries can make my life easier. Something like: |
I don't see why something like [Export] Godot.Collections.Dictionary<string, Texture2D> shouldn't work in editor; it's already supported in code, where it's least useful. I want to create a custom resource with character names as keys and a picture of the character as values. Without static type keys and values, I have scroll through every possible type of value to reach the bottom of the list to the "load" option in order to load a single texture2D for a single key:value pair. That's not sustainable for more than one or two textures, so dictionaries are essentially unusable for anything other than storing a few int/float/string pairs constructed in code. One of the key purposes of dictionaries is to store MANY things. |
Sounds great. It will give a choice and keep consistency. |
@mrpedrobraga, I think that we might not even need special case syntax here: |
I kinda undestand the logic behind doing sth like |
If/when this and/or structs are added, something very similar to the above text should be added to the Godot documentation. It greatly helps to clarify when to use each. |
We already use Variant in several other places, like parameters in functions for example. So i don't see why it would be any different here. |
I don't think that is a good idea, because underscore may be used to mark an "unused" argument in some languages. Even at the best day this would not be an unused argument. |
|
In the case you need |
It took me a while, but i've read all the thread to know what are we discussing at this point (After 2 years of participating in this thread). What I feel it's clear at this point is that Dictionary is a key-value system, and not a struct-like one. If you need a struct, a Resource is enough for most cases. Then, we need a disambiguation here...
In the case of Godot.... if the feature is plugged in the editor both the inspector and all languages (GDScript, C#, and other through connectors) could benefit from it. Maybe this is enough for most of the non-GDScript users here, and it's a reasonable feature to refine and deliver. In the case of GDScript, the problem with dictionaries and adding typing-hints to be type safe in GDScript is that you may need only keys or only values to be typed... There is no problem with However, as long as it seems to be open for discussion what syntax we should use.... My grain of salt is "please don't use Variant for variants"... Why? There's a funny quote of Uncle Bob that I like a lot that says "People keep coding Fortran in many other modern languages"... GDScript is the possibility of offering a modern spiced language that adapts specifically to the Godot gamedev needs and still look so good people says "I want to write code with this too" when they see me code. I believe a refreshing syntax will emerge. Maybe |
@chexmo You're looking for #7329, this is only for type-strong dictionaries. Also _ is not a type in any language and GDScript it absolutely would make no sense, Variant is the type that already does what you want _ to do inherently, that's not what placeholder variable names are for, even in C# and JS _ has never referred to a type, its a variable name that means "I don't care about this variable's value". |
Imo this is not the thread to be discussing renaming variants to ? or _. (I'm saying this because this would be a huge improvement to gdscript, and imo we don't need any more derailment in this thread) |
Would this proposal solve this issue about not being able to use a PackedScene as an exported Dictionary value? Or is that something else entirely? I was surprised to find that I couldn't add PackedScenes as exported Dictionary values already, as I was trying to make a simple database to help with loading scenes based on certain keys. |
After looking into the typed array I imagined that dictionaries would use the same pattern and this was already implemented. If I may ask, how can we help on this feature? |
At this time there isn't really anything that needs doing from any contributor, we're in feature freeze for 4.3 so it won't be merged right now, but hopefully it can be reviewed early in the 4.4 release cycle, it's a large feature so it's been hard for reviewers to find the time to focus on it properly Though testing the feature is always welcome, you'd need to check out the PR and rebase it and compile it yourself Edit: Though I recall there has been some discussion on what might need to be figured out before merging that might still be important, like the way generics works in GDScript and how to resolve that, things like casting typed arrays etc., which is currently an issue, and there's been discussion on whether to fix those first and then merge the typed dicts or fix that after merging |
This comment was marked as off-topic.
This comment was marked as off-topic.
@AThousandShips with Dev1 released for Godot 4.4, I noticed this feature was missing- I gotta say it's probably my most anticipated feature right now just for the inspector ergonomics/correctness. Would you happen to know the status of this? |
I'm not in any loops around this feature so I can't really respond sorry |
In case anyone is coming across this topic, I just compiled Godot from the |
Yeah, this is why it's marked as completed. 🤨 |
Describe the project you are working on:
Cutscene-heavy RPG
Describe how this feature / enhancement will help your project:
I want to associate some object with some identifier to make it easy to reference them from the AnimationPlayer (in this case it's some Dialogue class with data for who is talking and what they're saying.) I would like to access this data using the format
read_dialogue(id: String)
, where the id is the key in a dictionary that leads to the data. This causes my extended AnimationPlayer to pause the animation so it can autotype some text and wait for player input before moving on with the animation.However, there's currently no way to define a dictionary to have any particular kinds of keys and values, so if, for example, I wanted to add 50 different lines of text to read in this animation, I'd have to set the key's type and the value's type manually every time, which is tedious.
Furthermore: without dictionary hints, it's not possible to add a hint to a particular kind of Object, since the list of types you can pick from is limited to built-ins and various primitive types. Contrast this to arrays, where you can use hints to specify just about any type not present in the drop-down. So, you're not going to be able to easily create resources in-place, resulting in the OmniResource issue (except worse since it's every object ever.)
Show a mock up screenshots/video or a flow diagram explaining how your proposal will work:
export(Dictionary, String, Resource) var hinted_dictionary
If this enhancement will not be used often, can it be worked around with a few lines of script?:
I think just about everyone will be very pleased with this addition; however, in the meantime, it's not too bothersome to avoid the issue. Instead I'll give my class an
id: String
property and loop through an array. I assume this isn't as efficient as using dictionaries, but I likely won't notice a real performance hitch. Nonetheless, it's a shame exported dictionaries can't be more useful.godotengine/godot#25157
The text was updated successfully, but these errors were encountered: