Skip to content

SimpleFileBrowser upgrade breaks "new folder" creation functionality #2

Closed

Description

@TobiasWehrum

Description of the bug

After upgrading SimpleFileBrowser, making a new folder via the dropdown in the upper right corner doesn't work correctly anymore. See the last two reproduction steps for what is going wrong.

Reproduction steps

  1. Import SimpleFileBrowser into a project.
  2. Add Plugins/SimpleFileBrowser folder to the Assets & Scenes To Upgrade list and then upgrade everything.
  3. Call FileBrowser.ShowLoadDialog()
  4. Make a new folder via the upper right corner menu.
  5. Enter a name for the new folder.
  6. Press the return key: Nothing changes, the naming process UI stays open.
  7. Click on another file: The other file gets selected, but the new folder naming process UI still stays open.

Platform specs

  • Unity version: 2022.3.17f1
  • Platform: Windows 10
  • Device: Tested in the editor
  • How did you download the plugin: Asset Store (both assets)

Activity

  1. yasirkula commented on Jan 21, 2024

    @yasirkula
    Owner

    Thank you for the detailed bug report and sorry about the late reply. I've found out that the broken input field uses On Submit event in uGUI whereas that event doesn't have a corresponding serializable field in TMP. TMP does have an onSubmit event but for unknown reasons, it isn't serializable, so its value resets after entering play mode or quitting Unity. I've thought about two different solutions:

    • Appending uGUI's On Submit event to TMP's On End Edit event: although both events are similar, we're effectively changing the event type which may have undesired effects. Also, this means that uGUI's On Submit and On End Edit events will be combined together in TMP's On End Edit event.
    • Creating a script that just has a serializable UnityEvent for storing OnSubmit event and attaching it to upgraded InputFields. Then, the script will assign that OnSubmit event to the TMP_InputField in Awake. This requires the script to either come bundled with this TextToTMP plugin (simpler but if user deletes the plugin, the script will be lost which is very bad) or the script needs to be generated at Assets folder during the upgrade (afterwards, the user can move the script) (this is a more complex solution but possibly the only viable solution).

    What do you think about this situation?

  2. TobiasWehrum commented on Jan 21, 2024

    @TobiasWehrum
    Author

    What do you think about this situation?

    I agree that you probably shouldn't replace events, however similar they might be - at least not without user input. The generated script from the second version seems like the way to go.


    Unrelatedly to the functionality of UnityTextToTextMeshProUpgradeTool: I'm a bit confused as to why Unity Simple File Browser isn't using uGUI's On End event in the first place. I think it would make for a better user experience if the (re)naming was submitted when the field loses its focus, e.g. when the user selects another element afterwards. That's also what the Windows Explorer does.

    (And in general, I agree with yasirkula/UnitySimpleFileBrowser#82. Having to use a tool to convert legacy components in a plugin feels a bit clunky and makes updates harder.)

  3. yasirkula commented on Jan 23, 2024

    @yasirkula
    Owner

    Actually, in Unity 5.6 where the plugin was developed, I've noticed that the event is correct. I guess Unity have renamed the event from On End Edit to On Submit at some point but then decided to add On End Edit back, causing this strange phenomenon.

    image

    image

  4. TobiasWehrum commented on Jan 23, 2024

    @TobiasWehrum
    Author

    Wow, that's a wild FormerlySerializedAs chain. Guess they were less scrupulous about mixing up events that do different things...

  5. yasirkula commented on Feb 10, 2024

    @yasirkula
    Owner

    I've fixed the issue on SimpleFileBrowser-side with v1.6.5 release but will also update this plugin to properly support On Submit event.

  6. yasirkula commented on Feb 18, 2024

    @yasirkula
    Owner

    I've finally fixed this issue. I had a busy schedule and I've encountered another major issue along the way (f1ca6ce), so 🤷‍♂️

    I didn't go along with the procedural script approach. I've instead embedded the bridge script in the plugin and added a remark in https://github.com/yasirkula/UnityTextToTextMeshProUpgradeTool#known-limitations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions