godot icon indicating copy to clipboard operation
godot copied to clipboard

Add `PROPERTY_HINT_INPUT_NAME` for use with `@export_custom` to allow using

Open Dynamic-Pistol opened this issue 1 year ago • 10 comments

Based off https://github.com/godotengine/godot-proposals/discussions/7559, this adds a new property hint for use, which creates an enum suggestion for Strings/StringNames alongside an option to allow "show_builtin" to show builtin actions, docs are also included.

Note: this is a remake of another pr due to issues with github and me, and the fact it originally had an annotation

Dynamic-Pistol avatar Sep 05 '24 15:09 Dynamic-Pistol

You seem to have changed a bunch of GDScript output files now, please restore them

AThousandShips avatar Sep 05 '24 15:09 AThousandShips

You seem to have changed a bunch of GDScript output files now, please restore them

weird, i did initally as somehow the utils.notest.gd file was screwed,and while generating the tests, they got renewed, but i reverted the commit, so they shouldn't be merged? look at changed files tab

edit: nvm some got in somehow

Dynamic-Pistol avatar Sep 05 '24 15:09 Dynamic-Pistol

You removed the revert in your latest push for some reason, you probably have an out-of-date version of the branch locally

Also, in the future, make sure to create a new branch for each PR, you're contributing from master which is error prone

AThousandShips avatar Sep 05 '24 15:09 AThousandShips

You removed the revert in your latest push for some reason, you probably have an out-of-date version of the branch locally

Should be fixed now

Also, in the future, make sure to create a new branch for each PR, you're contributing from master which is error prone

Got it

Dynamic-Pistol avatar Sep 05 '24 15:09 Dynamic-Pistol

@AThousandShips for some reason, the macos build failed unit tests with utils.notest.gd even after i fixed it?

Dynamic-Pistol avatar Sep 05 '24 16:09 Dynamic-Pistol

Not sure, you might not have registered the hint correctly

AThousandShips avatar Sep 05 '24 16:09 AThousandShips

This is probably because the new constant is out of order, as PROPERTY_HINT_LAYERS_AVOIDANCE isn't exposed, meaning that the enum is missing an entry probably, though I'm not sure

AThousandShips avatar Sep 05 '24 17:09 AThousandShips

This is probably because the new constant is out of order, as PROPERTY_HINT_LAYERS_AVOIDANCE isn't exposed, meaning that the enum is missing an entry probably, though I'm not sure

So how would i fix it?

Dynamic-Pistol avatar Sep 05 '24 17:09 Dynamic-Pistol

I don't know, try copying the enum from utils.notest.gd and open it in the editor and see if it spits out any errors and go from there

AThousandShips avatar Sep 05 '24 17:09 AThousandShips

You've made merge commits, you need to push with git push --force or you'll merge the changes

I suggest squashing your changes as you have a lot of unnecessary commits in this PR, and you need to fix the branching issues you're having and fix the formatting

AThousandShips avatar Sep 05 '24 18:09 AThousandShips

  • If you don't set a default value (and the property is null, no dropdown appears to select a custom input action you created (or built-in actions if show_builtin is used):

This seems to be an issue with EditorPropertyTextEnum (it also happens with it), it seems to have to do with the object being variant with a value of null, only solution i can think of is to set the object to either the first input value or an empty string, but i don't feel like that's right, and i could try to fix the issue withe the text enum prop editor, but it seems out of scope for this pr, so either the solution i mentioned (if possible) or i just let it be someone else's problem

  • If you create an exported input action property, the property has a default value assigned and the project has no custom properties, the property's height will be very small (lower than all other property types):

"the project has no custom properties" uhh what custom properties? like no input actions? be more clear, also sounds like another EditorPropertyTextEnum issue, not sure if you can remake it with XXX_ENUM_SUGGESTION but it still would be the source problem, again this goes out of scope for this pr

  • Having the hint string separated by a space (loose_mode, show_builtin instead of loose_mode,show_builtin) causes it to break:

Okay, it should be fixed with a simple Vector<String> hints = p_hint_text.remove_chars(" ").split(",", false);

  • Having a typo in the hint string (e.g. show_builti instead of show_builtin) won't print any errors to the editor. This should be added if possible.

I don't think this is feasible with a property hint, none of the hints have errors when you use @export_custom, i suppose this could work

for (const String& hint : hints) {
        if (hint != "show_builtin" || hint != "loose_mode") {
            ERR_FAIL_V_MSG(get_input_action_editor(""), "Invalid PROPERTY_HINT_INPUT_NAME with hint \"%s\"");
        }
    }

but i want you to decide with others if this is really what is needed


Okay this should be done, @Calinou there shouldn't be any issues, i will push these changes tomorrow since i am going to sleep at the time this is written

some others changes include

  • Adding another comment for future ref
  • Adding a is_string_name parameter because in the old code the 2nd parameter of setup was always false for some reason

Have a nice day

Dynamic-Pistol avatar Mar 22 '25 00:03 Dynamic-Pistol

"the project has no custom properties" uhh what custom properties?

Yes, I meant custom input actions defined in the input map.

Calinou avatar Mar 22 '25 00:03 Calinou

Thanks! Congratulations on your first merged contribution! 🎉

Repiteo avatar May 13 '25 21:05 Repiteo