yairm210 / Unciv

Open-source Android/Desktop remake of Civ V
Mozilla Public License 2.0
8.51k stars 1.57k forks source link

Load from Custom Location crashes desktop version #6902

Closed SomeTroglodyte closed 1 year ago

SomeTroglodyte commented 2 years ago

Platform Mint 20.3 running desktop from Studio

Version Current master

To Reproduce Use Load from Custom on desktop to load a save that would load fine when using the normal way. CTD bypassing our crash handlers.

Describe the bug Threading control off I would say. May have regressed with lwjgl3, which is far less threadsafe on Music than lwjgl2 as we learned, so why not on AWT interop as well... Or it regressed with the move to coroutines. Will know soon.

console ``` X Error of failed request: BadWindow (invalid Window parameter) Major opcode of failed request: 20 (X_GetProperty) Resource id in failed request: 0x8a00024 Serial number of failed request: 3424 Current serial number in output stream: 3424 ```

Additional context Just so you know and it's searchable. Been on my list for a while, will likely fix with Save/LoadGameScreen rework.

JackRainy commented 2 years ago

It works fine for me with the latest master running Windows 10 and starting Unciv from the Android Studio. Did you try git bisect to find the commit since the problem appears? Don't you even see the open dialog prompt? Probably, the reason is some 3rd party which provides the open dialog window.

SomeTroglodyte commented 2 years ago

fine for me with the latest master running Windows 10

Good to know, and condolences :smiling_imp: .

bisect

I have no clue with git CLI. I would have an idea how to find the "point of regression", but I prefer to try a direct fix first. The way it is doing asynchronous is suspect. Later!

Don't you even see

Sorry for being unclear ("load a save that would... implies weakly that I had a choice of files"). Yes, the classic AWT file dialog appears, it goes to Nirvana after selecting and closing that dialog. And of course AWT and Swing are 3rd party - there's parties enough :partying_face: .

JackRainy commented 2 years ago

the classic AWT file dialog appears

Good to know that at least this part works :)

and condolences 😈 .

Smells like a declaration of the Holy War... πŸ‘ΌπŸ» Btw, good idea for yet another Unciv mod with additional nations: Linux, Windows, MacOS. :trollface:

SomeTroglodyte commented 2 years ago

nations: Linux, Windows, MacOS

Yea! And VMS, RSX-11 and IBM i! The Nation unique of Windows could be "When a [military] unit is killed, it kills all [enemy units] in a [1] tile radius and adds [50]% of the kill XP to the nearest [Bill Gates]", with "Bill Gates" being the unique Great General. Linux could be "Adds [250%] of current overflow Science to [all other] Civilizations ".

SomeTroglodyte commented 2 years ago

Welp, if a basic system library allows a crash that kills the process without a chance to catch it first, then the library and the system (java ecosystem) are cr...ud.

I'm pretty sure this worked when designed (#3160) by @wbrawner, and that was lwjgl2 times. Too high for me.

Team decision: drop the CustomSaveLocationHelperDesktop? Hide the access on Linux? Can any other Linuxers load from custom - or even cancel the dialog without crashing?

OptimizedForDensity commented 2 years ago

I'm on Linux and crash with the same error even just clicking the "Load from custom location" button then clicking cancel on the popup file selection window.

SomeTroglodyte commented 2 years ago

Any Linuxer with Wayland?

linuxpng commented 2 years ago

I am running Wayland on Fedora 36 Gnome 42. I came looking for this to see if it had been reported.

Flatpak from flathub: UnCiv io.github.yairm210.unciv 4.1.3 stable system

I hope the feature isn't removed, I was really hoping to have the ability to pick up and play between android and linux devices with saves from Google Drive.

SomeTroglodyte commented 2 years ago

... and does the feature work for you?

JackRainy commented 2 years ago

I came looking for this to see if it had been reported.

That sounds like it does not work.

SomeTroglodyte commented 2 years ago

True, sounds like. Should have asked for console output, as for me there's the explicit "X" there, but I asked for Wayland because Wayland is not X11, or is it?

Anyway, I fear we'll lose this battle. It's a thing of conflicting UI library coexistence, and if you go read lwjgl3 sources and what they say on awt/swing you won't get a hopeful feeling. Actually, the whole ecosystem smells to me like a jumbled card house only waiting to topple on the slightest wind gust. What we should do -maybe- is escalate upstream asking for help, and I don't have the energy.

SomeTroglodyte commented 2 years ago

@ All, @Azzurite : Shall we adapt this dialog to our look and feel and use it for desktop? Will "document providers" always have a path representation on desktop variants? (They could - 'nix no worries, and 'doze has reparse points for just that kind of usecase, nonwithstanding the coder landscape being probably too - let's say low end to know by now...)

Edit: Warning: link leads to crappy site, don't visit without a good blocker, and don't even hover over that code pane 'share' icon - it will hang the browser tab.

Azzurite commented 2 years ago

hall we adapt this dialog to our look and feel and use it for desktop

I'm sorry, I don't care enough about this topic :) I would leave it to your discretion on what to do.

MayeulC commented 2 years ago

I am running Wayland on Fedora 36 Gnome 42. I came looking for this to see if it had been reported.

Flatpak from flathub: UnCiv io.github.yairm210.unciv 4.1.3 stable system

I hope the feature isn't removed, I was really hoping to have the ability to pick up and play between android and linux devices with saves from Google Drive.

Works for me. Flathub version, Arch, sway/Wayland. I only selected the option, which displayed a pop-up, and selected an existing save file, which got me in-game. I did not try importing a save game from another platform.

SomeTroglodyte commented 1 year ago

Outdated