Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Geyser window placement bug #2076

Closed
basooza opened this issue Oct 23, 2018 · 26 comments · Fixed by #2945
Closed

Geyser window placement bug #2076

basooza opened this issue Oct 23, 2018 · 26 comments · Fixed by #2945

Comments

@basooza
Copy link
Contributor

basooza commented Oct 23, 2018

Brief summary of issue / Description of requested feature:

Story time. I came back to Mudlet and discovered an oddity in a UI I'd written a couple months ago. The code creates a series of Geyser labels and miniConsoles with parent-child relationships that all roll up to a single Geyser Container. However when the system initializes and creates this UI, some elements would be displaced. The same elements every time, always by the same suspiciously even amounts (eg 50% of the width of the container and 10% of its height) that were used as constraints in nearby code. Weirdly, despite being visibly misaligned, Mudlet didn't seem to believe the elements were where my eyes said they were. Examining the Geyser constraints via the x & y properties on the items said that the elements were in their proper locations. Calling :flash() on the elements caused flashes that were displaced from the visible elements that the method was being called on. Even more weird was the fact that I could fix all of this with the following line of code:

parentContainer:move(parentContainer.x, parentContainer.y)

... literally just moving the topmost container to where it should already be, at which point suddenly all the misaligned elements would sort themselves out (but it wouldn't work immediately after creating the elements, I had to run it from the command line or aliases). An alternative way to fix it was by resizing the Mudlet window, at which point everything snapped into place.

All of this could still be the purview of some horrifying code I cooked up (and still might be), but after all this I was bothered by the nagging feeling that when I'd written this UI a couple months ago it didn't suffer from this bug, and so I reinstalled past versions of Mudlet and loaded a profile with the offending code to test. As it turns out this bug (which is 100% reproducible with my code) is only present AFTER version 3.10.1 (possibly also after 3.10.2, but I couldn't find a Windows installer for that version). The bug (or changed behavior) was introduced sometime between 3.10.1, which worked, and 3.11.0, which did not. I also verified that 3.13.0 and 3.12.0 also suffer from this issue.

I will try to narrow down the offending snippet and provide it here, but I'm out of time and will have to tackle it another day.

@vadi2
Copy link
Member

vadi2 commented Oct 23, 2018

I think the issue might be with getMainWindowSize() which got broken with a Qt upgrade, which would explain the symptoms of us not having touched Geyser core in ages but it breaking recently (Qt version change). See if that function is the culprit for you.

@vadi2
Copy link
Member

vadi2 commented Oct 23, 2018

@SlySven
Copy link
Member

SlySven commented Oct 31, 2018

👍 I'm not sure I can contribute to this discussion at the present with my RL as it is, but can I just thank you @basooza for what I think is a constructive issue report - I get the feeling that there is a lot of relevant details there that will be helpful especially as to narrowing down which versions manifested the issue first...

@vadi2 vadi2 added this to the 3.17.0 milestone Dec 22, 2018
@vadi2 vadi2 modified the milestones: 3.17.0, 3.18.0 Feb 2, 2019
@vadi2 vadi2 self-assigned this Mar 11, 2019
@vadi2 vadi2 modified the milestones: 3.18.0, 3.19.0 Mar 16, 2019
@Jieiku
Copy link
Contributor

Jieiku commented Mar 18, 2019

I just wanted to add some info here after testing. When I first launch Mudlet on Windows 8.1 and my windows placement is incorrect. I can run lua display(getMainWindowSize()) and it indeed shows incorrect values. To fix this I un maximize the window then drag the bottom right corner to resize the window, then put it back to maximized.

I also found that you can toggle the Main Toolbar visibility, and that will fix it as well! So it seems anything that affects the MainWindowSize is enough to fix this issue. I am wondering if we could as a workaround quickly hide/show the Main Toolbar visibility or something. That would likely fix it until the issue with QT is resolved.

2019-03-17_23-29_1

2019-03-17_23-30

2019-03-17_23-31

@vadi2
Copy link
Member

vadi2 commented Mar 18, 2019

Give it a go!

@Jieiku
Copy link
Contributor

Jieiku commented Mar 22, 2019

I have more info to provide. instead of resizing the window. you can just hit the close button for Mudlet in top right corner, and answer Yes to save profile. On the next launch this weird placement will not happen.

So basically its every other launch that you see this issue. I am going to go out on a limb with this new discovery and say that it might not be QT that is responsible, otherwise why it would not fix/be correct on every other launch. My guess is it has to do with something that is saved to the profile, maybe comparing the profile data from both states would shed some light.

I am going to see if I can debug this any farther....

@Jieiku
Copy link
Contributor

Jieiku commented Mar 31, 2019

Well, I have not yet digged into the process of figuring out why this happens, but with continued use of Mudlet I have figured out how to reproduce it Every Time. First there seems to be multiple things that can trigger this bug, they all seem to be tied to issues with the profile properly loading/saving. This particular one is just how i found to reproduce it every time.

On a windows system, first have the setting from the games->Play menu "Open profile on Mudlet start" Unchecked. To do goto the menu Games -> Play, uncheck the box and hit connect. Once the profile open you can close Mudlet, save profile, then launch mudlet again.

At the point mudlet should be configured so that it does not automatically launch a profile.

Now here is the next part, If you launch Mudlet and then in the "Character name" field fill in a different character name (such as an ALT character). And hit the "connect" button. You will see that window placement is all wrong. At this point, the easiest way to fix the bug is to close Mudlet, save profile, then relaunch mudlet. If you log into the same character then the bug will be gone.

To reproduce the bug all you have to do is close mudlet and enter a Different character name again, and your back to improper window placement, relaunch and its fixed.

I think I have done a good job of describing how to reproduce this bug, but if anyone is having trouble following what I am saying I would be happy to make a video that shows me reproducing this bug. Just let me know.

@SlySven
Copy link
Member

SlySven commented Apr 1, 2019

Could the code that saves the (docked: mapper/user windows/toolbars) windows layout just be borked?

I've always been a bit unclear how it could be possible to save the layout data when there could be multiple profiles loaded (each possibly with a docked mapper widget, with their own toolbars and with other docked user windows) and then expect when one of those profiles gets reloaded for the data for all of those to be interpreted and then also respected when a second or more other profiles are loaded...

@SlySven
Copy link
Member

SlySven commented Apr 1, 2019

... of course any, or, all of those sub windows can be merged-docked(?) into a tabbed docked window (effectively docked on top of each other) and then what does the layout/positing code make of THAT?

@vadi2
Copy link
Member

vadi2 commented Apr 1, 2019

@TheFae would you know?

@Jieiku
Copy link
Contributor

Jieiku commented Apr 1, 2019

I will just add to this, that it only happens to me on windows, and can be easily reproduced on windows. I have never once seen this happen on linux, I am unsure why linux is happy with how the profile is but windows is not. As soon as I can get some free time I will do my best to figure this one out, my idea was to compare the profile data between a session where the bug occurs and a session where it does not occur and look for discrepancies. I just have not had enough time to really dig into this one yet.

@vadi2 vadi2 removed this from the 3.19.0 milestone Apr 7, 2019
@Jieiku
Copy link
Contributor

Jieiku commented Apr 9, 2019

I compared the profile saved settings and nothing looks different between before and after the load of mudlet when the error occurs, I am wondering what other files might be responsible such as windowLayout.dat (if this was an xml file comparing would be easy, lol)

however that file is encoded and so I am not sure how to compare it before/after @vadi2 do you know how I can view the contents of this file before/after a load when the error occurs. I would really like to track down and fix this bug.

@vadi2
Copy link
Member

vadi2 commented Apr 9, 2019

@itsTheFae would you know?

@vadi2
Copy link
Member

vadi2 commented Apr 9, 2019

@Xekon that file got added in Mudlet 3.2 when userwindows started remembering their position: https://www.mudlet.org/2017/06/mudlet-3-2

Want to test if this issue exists in Mudlet 3.1 and 3.2?

@Jieiku
Copy link
Contributor

Jieiku commented Apr 9, 2019

In 3.1 my profile does not load in a maner that I am used too, i suppose window placement was handled differently then.

However based on the first post of this report they mentioned 3.10.1 did not have the issue. I have been able to reproduce the bug 100% every time and can confirm that it did not exist is 3.10.1

I continued to test upward from: 3.11, 3.11.1, 3.12, and then finally 3.13 showed me the bug.

so to further test I decided to try downgraded in order starting with 3.12! and the bug is present, even though when I upgraded incrementally from 3.11 to 3.12 it did not happen.

so I downgraded further to 3.11.1 bug still there.

next 3.11, bug still there.

next 3.10.1 and finally the bug cleared up.


to summarize 3.10.1 does not appear to have the bug at all.

After that you can upgrade from 3.10.1 to 3.11, 3.11.1, or 3.12 without experiencing the bug, but as soon as you upgrade to 3.13 you will experience the bug.

After having experienced the bug with 3.13 or later, the only way to completely clear the bug up is to downgrade to 3.10.1 (3.11, 3.11.1, or 3.12 will not clear the bug up until you first downgrade as far as 3.10.1)


My gut feeling is that something in the windowLayout.dat file is being saved improperly. (and somehow this does not affect linux, but it does affect windows.)

@vadi2
Copy link
Member

vadi2 commented Apr 9, 2019

Can you try clearing windowLayout.dat between upgrades 3.10.1 - 3.13?

@Jieiku
Copy link
Contributor

Jieiku commented Apr 9, 2019

If I clear the windowLayout.dat file The bug presents itself immediately, with 3.11-3.13.

Seems 3.10.1 was the last version that did not display the bug.

@vadi2
Copy link
Member

vadi2 commented Apr 9, 2019

So 3.11 is the dodgy release. https://www.mudlet.org/2018/07/mudlet-3-11-quality-improvements-all-around are there release notes for it and Mudlet-3.10.1...Mudlet-3.11.0 is the diff (unfortunately a lot of noise when we removed unused 3rdparty/zziplib)

@vadi2
Copy link
Member

vadi2 commented Apr 9, 2019

100% sure it works OK with Mudlet-3.10.0? That's when we changed from Qt 5.6 to 5.11 (that we still use to this day). Could have been a culprit in there and not in Mudlets code. It's what I suspected originally in #2076 (comment)

@Jieiku
Copy link
Contributor

Jieiku commented Apr 9, 2019

Yes 3.10.1 has zero issues.

I would agree about it being Qt except that the bug only occurs every other launch when changing the character name used to access a profile which suggests it has something to do with how things are saved... That was why I wanted to dig into the values stored in the windowLayout.dat

It could also be that the newer version of Qt is saving/loading values differently than it did in the older version of Qt, but in either case I need a starting point to dig in and see what is going on. I need to be able to check the values being used vs the values being stored and figure out what the root cause is.

I will have to try and look in the code for all occurrences of windowLayout.dat and see if I can figure out the best way to debug this :) (I am kinda thinking when it saves values to that file, also have it store the values to a flat .txt or xml file, so that I can get in there and look)

@vadi2
Copy link
Member

vadi2 commented Apr 9, 2019

Yep. Maybe playing around with https://wiki.mudlet.org/w/Manual:Miscellaneous_Functions#saveWindowLayout and https://wiki.mudlet.org/w/Manual:Miscellaneous_Functions#loadWindowLayout will yield some results too.

@Jieiku
Copy link
Contributor

Jieiku commented Apr 10, 2019

Thanks to Caled on discord, I found out this bug is likely related: #2023

Caled also said: "Also, my workaround has been to create the mapper and add to GUIframe at the 'sysLoadEvent' event. Just in case that information is also useful"

This example uses Jor'Mox GUIFrame and when the mapper is enabled the bug is present:
bugged.zip

here is a bug free example that has a mapper:
bugfree.zip

here is a gif show the bug in action after importing the bugged script (you will see on the later launch of this profile I disable the loading of the map, which seems to prevent the bug.):
bug_layout_map

here is a gif showing proper function of the bugfree script that even has a mapper present:
bugfree_withmap

@Jieiku
Copy link
Contributor

Jieiku commented Apr 11, 2019

With basooza help I I got this working so that the bug does not happen. It seems the bug happens as a result of moving/interacting with the map prior to Mudlet fully loading, and obviously only under certain conditions because I do have a bugfree example above.

The way GUIFrame draws all the Ui elements makes the map bug very obvious, but if you delay interacting with the map until mudlet is fully loaded then it will not happen.

ALSO! this bug only happens on Windows, which means that Linux is smart enough or naturally loads faster, so interacting with the map is not an issue. However on windows you need to delay interacting with or moving the map for about 100ms to avoid the bug.

here the solution I came up with thanks to @basooza:
bugfix_final.zip

Another thing you will notice in the screenshot below I print() the result of getMainWindowSize() and you can see that the bug still happens, but that using a tempTimer of atleast 100ms is working around it:
bugfix_finals

Also to add to this..... simply having a tempTimer is not enough, because I tried a time of 0.01 and that was not enough (10ms) but 0.1 is enough (100ms)

@Jieiku
Copy link
Contributor

Jieiku commented Apr 11, 2019

@vadi2 requested a VERY simple script to reproduce the bug, here it is:
bug-getMainWindowSize.zip

and a screenshot showing the print() debug:
2019-04-11_04-09

mapperContainer = Geyser.Container:new({x=-700,y=0,width=700,height="70%",name="mapperContainer"})

print('Before Geyser.Mapper:new: ' .. tostring(getMainWindowSize()))
mapper = Geyser.Mapper:new({name = "mapper", x = 0, y = 0, width = "100%", height = "100%"}, mapperContainer)
print('After Geyser.Mapper:new: ' .. tostring(getMainWindowSize()))

print('before 100ms tempTimer: ' .. tostring(getMainWindowSize()))
tempTimer(0.1, function()
	print('after 100ms tempTimer: ' .. tostring(getMainWindowSize()))
end)

@vadi2
Copy link
Member

vadi2 commented Aug 6, 2019

Working through this issue. It would seem the left/right toolbars which is where the easy buttons usually go somehow gain width for a moment - and that is why it's reduced here.

@vadi2
Copy link
Member

vadi2 commented Aug 7, 2019

@basooza @Xekon please test if #2945 fixes the issue you describe here. Thank you!

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

Successfully merging a pull request may close this issue.

4 participants