search

yairm210 / Unciv

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

Mod previews aren't working on Windows #9854

Closed Caballero-Arepa closed 1 year ago

Caballero-Arepa commented 1 year ago

imagen

I came back and I can't see the mod preview, but on phone I can.

yairm210 commented 1 year ago

What version are you on?

Caballero-Arepa commented 1 year ago

Sorry, 4.7.10.

yairm210 commented 1 year ago

Hm, working on my Ubuntu, I see you have Windows but I do wonder if that would make a difference 🤔

maliprinc420 commented 1 year ago

I have the same problem (on Windows11).

I also noticed that resizing the window while being in the mod interface actually hide the mod infos and you have to close and reopen the interface to see it again.

Screenshot ![image](https://github.com/yairm210/Unciv/assets/47052682/79be14e9-bf02-4dd1-9efa-7ec199e2d99a)
SomeTroglodyte commented 1 year ago
Works on an older W10 (20H2 I believe) ![image](https://github.com/yairm210/Unciv/assets/63000004/93fdf0c5-cd3a-4f88-8f25-b7d985583cff)

.. the resizing thing, reproduce I can.

SomeTroglodyte commented 1 year ago

...The resize tries to keep mod metadata, but these maps do contain the button widgets, including their listeners. The refresh routines fail to rebind onClick to use the new screen in closures, 'cuz - see "Prevent building up listeners" comment. At least that's the reason the left column fails. Center column is likely similar.

Mmgr needs a major overhaul - one that separates UI from data better, avoids code duplication better (one modActionTable updater), and maybe cleans some completely outdated comments - like #8648 removing lines but not bothering to see there's a comment referring to them... Or the comment why setDefaultCloseAction isn't used is way outdated - obsoleted by the screen stack...

Too lazy. I may PR a fix to that, if I see a cheap and relatively elegant way. So far drawing blanks. Unelegant would be running clearlisteners on all the passed-in ModUIData.button's in init...

SomeTroglodyte commented 1 year ago

I came back

... from the dead? 🧟

But, amigo, dumb question: Is your local copy your work copy and maybe not quite comparable to what we get downloading it? Any symlinks? Is the local preview png fine? For me, the preview is quite resilient and I can even offer an invalid local file, it will still display the online one. Or did you simply run into github's rate limit?

maliprinc420 commented 1 year ago

I personnaly use the Unciv-Windows64.zip available at each new release and considering my usage of github I would be surprised if the problem is github's rate limit. But which local preview are you talking about?

SomeTroglodyte commented 1 year ago

I personnaly use the Unciv-Windows64.zip available at each new release and considering my usage of github I would be surprised if the problem is github's rate limit. But which local preview are you talking about?

"personally" -oops, sorry, did I type that out loud? 😉

The OP is a Mod author. So they might be modifying their local copy as source to later upload to their repository, and have structural specialities there - I'm not the only one symlinking a usable-by-unciv copy to a "source-as-seen-by-git" copy. Just an idea such stuff might hinder Unciv's preview display, as I have no Win11 box to test on. And a Mod author might - just might - open and close that mod manager often enough to trigger a rate limit.

You're saying you get missing previews too and are sure it's not a rate limit - OK, I'll believe you - thanks for the input corroborating it may be a Win11 (or a JRE associated with it) thing! - until I find a box with Win11 and Studio installed to actually test myself - which likely means practically never, as I'm not paying for a license for that (that money is reserved for supporting an OS I believe in, and "they" even get more than Win would cost in the long run), so "someone" has to put one on my desk for other reasons...

Now I mention it - which Java exactly is running on your (@Caballero-Arepa 's) box? The zips come with their own don't they.... (Oh my, a stone age Java 8, albeit a modernized one..) And my test was using an almost-current temurin openjdk (17.0.6). Might be relevant, or not.

Caballero-Arepa commented 1 year ago

... from the dead? 🧟 XD, nah it's just that I took a break from games to focus on irl stuff.

But, amigo, dumb question: Is your local copy your work copy and maybe not quite comparable to what we get downloading it? Any symlinks? Is the local preview png fine? For me, the preview is quite resilient and I can even offer an invalid local file, it will still display the online one. Or did you simply run into github's rate limit?

For the program, it's the normal instalation from github (ie: I downloaded the .zip and open it, then I update just by fownloading the .jar)

For the mod, no, in the screenshot I redownloaded the mod, so it isn't a work copy. But previews don't work for me with other mods, like say Tech Stoper, so it's not just one mod, but all of them.

I also waited around 3 minutes to see if it loaded but no. Maybe I should wait a bit more to see if it's a loading problem.

Tldr: The mod has not been modified locally, neither other mods with previews work.

Looking around, I see I have a Java version "1.8.0_51", thats what cmd tells me. Could it be that? Idk, because when it was added it worked fine for me.

SomeTroglodyte commented 1 year ago

So we need to debug on Win11. :shrug: Or using the Java 8 runtime, which I'm pretty certain the debugger won't run under. One of these two factors must make a difference. Still, since it hasn't come up earlier I'd tend to Win11. Or I could try running on Win10 with the zip's jre tomorrow... Anyway, getting the resize to work is a much more challenging and nevertheless accessible puzzle.

SomeTroglodyte commented 1 year ago

... it's not Win11, it's the JRE! I'm truly baffled/surpraized. Unpack zip, run exe, reproduces. Run the same jar with my default JRE, workz.

Edit: Replace the zip's JRE with the latest Adoptium 11 one (OpenJDK11U-jre_x64_windows_hotspot_11.0.20_8.zip) - works. Replace with latest Adoptium 8 one - fails. So it's something that improved from Java 8 to Java 11 we rely on.

SomeTroglodyte commented 1 year ago

... wow the Studio debugger actually accepts running the target under a paleontolithic jre... So our code actually throws a java.lang.NoSuchMethodError: java.nio.ByteBuffer.position(I)Ljava/nio/ByteBuffer; on Github.kt line 263... Reading this fine analysis and then this mentioning no hint on that problem at all I conclude: Java has a really crappy library. But - Fixable.

yairm210 commented 1 year ago

So what I gather from you is we need to update the JDK on this line:

https://github.com/yairm210/Unciv/blob/ad75ad916b7e597daa815d7a8af8f1642ce99979/.github/workflows/buildAndDeploy.yml#L199

SomeTroglodyte commented 1 year ago

So what I gather from you is we need to update the JDK on this line...

Overlap - see linked PR for both alternatives. Haven't really judged political correctness of Adoptium - or their redist license, but I know no other maintained builds of Java 11 I personally trust...

SomeTroglodyte commented 1 year ago

Update to solve that resizing issue mentioned by @maliprinc420 is in the works too. So far simply separating saved Gdx elements from saved metadata and reconnecting the dots has solved the observed bug. And I think it's less lines despite needing repeated imports for more classes.

Still, needs more work - a bit more architectural simplification, and solve the issue that preview + "tiny" display setting makes perm av checkbox invisible and inoperable... And maybe allow a resize during a running query to continue the query (by "page" number), and maybe re-think the "..." for "I'm still running" that's too hard to even see. And maybe a visual indicator for "rate limited"???