ephread / inkgd

Implementation of inkle's Ink in pure GDScript for Godot, with editor support.
MIT License
305 stars 33 forks source link

Problems with the settings panel #36

Closed dploeger closed 2 years ago

dploeger commented 2 years ago

Describe the bug When using the file selectors in the settings panel, the selected files aren't accepted. Instead, one can only set the paths directly in the text inputs.

Also, the mono selector button is missing an icon.

And mono isn't required for macOS (anymore)

To Reproduce Try to select a mono binary, the path to inklecate, an ink file or a target file. None of the file selectors work.

The compile button requires a mono path, even for macOS where it isn't required.

Expected behavior The selectors work and no mono is required for macOS.

Environment:

videlanicolas commented 2 years ago

Found it, the problem here is that InkFileDialog connects signal popup_hide and file_selected, but the order of signal firing events is popup_hide and then file_selected. Looking at the method called in popup_hide we can see that it disconnects the method for signal file_selected, so when the popup is closed the method is not called and the path is not populated.

I'll send a PR to fix this.

dploeger commented 2 years ago

Awesome, thanks!

ephread commented 2 years ago

@dploeger thanks for reporting the issues. @videlanicolas thanks for fixing them.

And mono isn't required for macOS (anymore)

There's always been a prebuilt binary of inklecate embedding the mono runtime for macOS. I originally forced the use of a Mono / .NET runtime on macOS because I was experimenting with them, but that was a huge mistake. 😅 It's confusing and it makes no sense for most users.

Now that inklecate for Linux also comes pre-packaged with the runtime, I'm just going to hide the ability to set the Mono path behind an 'advanced' menu and disable it by default on all platforms.

videlanicolas commented 2 years ago

We should close this bug, ot at least release a new version with this fix and then close the bug.

ephread commented 2 years ago

@videlanicolas 0.2.3 was released yesterday and is pending review on the Asset Library. I'm going to keep this issue open until #43 is merged, though.

If you're cool with it, I'll add you as a reviewer when it's ready!

dploeger commented 2 years ago

@videlanicolas 0.2.3 was released yesterday and is pending review on the Asset Library.

I've just published it there. 👍🏽

videlanicolas commented 2 years ago

I'll add you as a reviewer when it's ready!

Sounds good!