vasl-developers / vasl

Virtual Advanced Squad Leader
http://vasl.info/
GNU Lesser General Public License v2.1
66 stars 28 forks source link

6.6.9 overlays dragged from draggable overlays disappear over map until dropped #1857

Closed jrvjrv closed 4 days ago

jrvjrv commented 2 weeks ago

steps to reproduce 0) preferences: combined application window 1) open new game, board 00 2) open draggable overlays window; move window off main window 3) drag overlay (I use one residual fp overlay) to map. 4) as soon as it leaves the draggable overlays window the overlay image disappears. The mouse cursor does not. 5) when it is released/dropped on the map it reappears. 6) if you drag the placed draggable overlay it does not appear on the map but is drawn in the upper-left corner of the main map window. When dropped the image appears in the drop location but the "ghost" remains in the upper-left.

vassal: 3.7.14, vasl: 6.6.9, java: openjdk version 23 OS: linux 6.11.5-amd64, distro debian trixie (testing)

jrvjrv commented 2 weeks ago

An additional issue, which is probably related somehow. If "use combined application window (requires restart)" is switched off, a counter dragged off the counter window disappears and does not reappear until dropped.

jrvjrv commented 2 weeks ago

A third issue, again probably related. Whether or not "combined application window" is engaged, when moving a counter from the map board to the map board by mouse drag-and-drop the counter does not move with the mouse and results in a "ghost" in the upper-left of the window after drop. Moving by ctrl-numpad does not result in a "ghost."

geezer09 commented 1 week ago

I opened up another issue and as Jrvjrv pointed out, it is probably one and the same. I am posting what I found here and deleting the previous issue:

The following two methods in ASLPieceMover were commented out recently just before releasing releasing version 6.6.9, probably to address a bug but i am not sure. This dosent seem to cause any issues in windows but in linux it screws up the click and drag function when moving pieces.

@Override
protected void moveDragCursor(int dragX, int dragY) {
    if (drawWin != null) {
        dragCursor.setLocation(dragX - drawOffset.x, dragY - drawOffset.y);
    }
}

/**
 * Removes the drag cursor from the current draw window
 */
@Override
protected void removeDragCursor() {
    if (drawWin != null) {
        if (dragCursor != null) {
            dragCursor.setVisible(false);
            drawWin.remove(dragCursor);
        }
        drawWin = null;
    }
}
derimmer commented 1 week ago

@jrvjrv @geezer09

Between posts of GS, FB, Discord and here, I managed to get myself totally confused about what has or has not been about the various bugs in 669.

Now that I am home again, I think I have it clearer in my head.

Of first importance are the linux bugs that I re-introduced by commenting about the two methods Joseph flagged above. I am going to undo that change and produce a module (6.6.9.1) that I will send to the two of you for testing via a dropbox link. I am hoping that undoing my change will fix ALL of the linux bugs but we will have to test that.

In addition, the SAVE AS PNG functionality seems to have gotten broken again. I will try to fix this. With any luck the fix will be in the version I send you for testing.

To my mind, these are the only two changes that need to be in an urgent release. Joseph has done a lot of great work on other bugs and features. I would rather hold those for 6.7.0-beta1. Any concerns with that?

geezer09 commented 1 week ago

Yeah I 100% agree not to put any of my recent changes in this hotfix. My only question is why not 6.6.10 instead of changing the version number format.

uckelman commented 1 week ago

Please, please, please do not use four-part version numbers. Three-part is what virtually everyone uses now. Make the next release 6.6.10.

derimmer commented 1 week ago

Here is the link to 6.6.10 for testing: https://www.dropbox.com/scl/fi/87vnb5lytincv1acnbvkg/vasl-6.6.10.vmod?rlkey=zj2kbpcp7grm57tixopplxytu&dl=0

I have no facility to test Linux changes so please test for all Linux related bugs described above.

If you would be so good as to test the SAVE AS PNG functionality, I would appreciate that very much. This is NOT Linux related as far as I can tell. However several people have reported it as being broken in 669 (after being fixed quite recently).

If we don't find any problems, this is good to go.

I see the posts about version numbers and have complied.

There will be lots of grumbling from the userbase about too many goddamn versions. It was my mistake that brought this about so I will wear it.

derimmer commented 1 week ago

@uckelman In July of this year, you pushed changes to add two methods (ASLPieceMover.DragHandlerNoImage.moveDragCursor() and ASLPieceMover.DragHandlerNoImage.removeDragCursor() to fix similar dragging bugs that Linux users were experiencing.

This seemed to work fine in 6.6.9-beta7 and beyond, which I believe were built with VASSAL3.7.12.

VASSAL3.7.14 issued in August and at some point I started to use it to build 6.6.9. However, when I did so, it produced error messages in my IDE noting that the two methods above were overriding methods in ASLPieceMover.AbstractDragHandler that were marked as Deprecated. I chose to comment out the two methods in ASLPieceMover.DragHandlerNoImage, which removed the errors from my build and I then issued 6.6.9.

What I have now done is to restore the two methods in ASLPieceMover.DragHandlerNoImage and then comment out the @Deprecated from the methods in ASLPieceMover.AbstractDragHandler. This removes all errors in my environment but needs to be tested in Linux.

It feels like a bad solution to me. We now have two methods that are marked for removal in VASSAL's version of PieceMover but which we will continue to use in VASL's ASLPieceMover.

Is there a better way to handle this?

You have warned us before about the consequences of overriding the entire PieceMover class which was done to get BoardZoomer to work. It seems like we are paying the price here.

geezer09 commented 1 week ago

I did a very quick test on ubuntu and didnt seem To have the bug with pieces not dragging properly.

derimmer commented 1 week ago

@jrvjrv have you had a chance to test drive this yet?

jrvjrv commented 5 days ago

Here is the link to 6.6.10 for testing: https://www.dropbox.com/scl/fi/87vnb5lytincv1acnbvkg/vasl-6.6.10.vmod?rlkey=zj2kbpcp7grm57tixopplxytu&dl=0

I have no facility to test Linux changes so please test for all Linux related bugs described above.

If you would be so good as to test the SAVE AS PNG functionality, I would appreciate that very much. This is NOT Linux related as far as I can tell. However several people have reported it as being broken in 669 (after being fixed quite recently).

If we don't find any problems, this is good to go.

I see the posts about version numbers and have complied.

There will be lots of grumbling from the userbase about too many goddamn versions. It was my mistake that brought this about so I will wear it.

This works for items dragged and dropped from the same combined window, e.g. counters when dragged with "combined application window" checked. If from a different window, e.g. counters when "combined application window" is not checked or draggable overlays regardless of "combined application window" the item being dragged disappears as soon as it leaves its source window and does not reappear until dropped on the target window. There are no "ghost" images in the upper-left, which is an improvement, and once dropped in the target window the items can be picked up again and moved normally, i.e. an image follows the mouse. Overall it's better but not 100%.

derimmer commented 4 days ago

This works for items dragged and dropped from the same combined window, e.g. counters when dragged with "combined application window" checked. If from a different window, e.g. counters when "combined application window" is not checked or draggable overlays regardless of "combined application window" the item being dragged disappears as soon as it leaves its source window and does not reappear until dropped on the target window. There are no "ghost" images in the upper-left, which is an improvement, and once dropped in the target window the items can be picked up again and moved normally, i.e. an image follows the mouse. Overall it's better but not 100%.

@geezer09 could you take a look at this using your ubuntu config?

geezer09 commented 4 days ago

Yeah I can confirm that the counters disappear when using multiple windows on ubuntu. I never actually played vasl on linux so I’m not sure if this is new or has always been like that. This is pretty minor in my opinion and is very edge case, pieces behave normally once on the map.

@jrvjrv, can you confirm if this is new behaviour or was like that in the past as well?

I wont have time to look at this for a week or so, its your call Doug if you want to wait or release the current fix.

derimmer commented 4 days ago

Over on Discord I asked @jrvjrv to tell me the last version in which the "disappearing drag image" bug did not occur. Here is his reply:

vasl 6.6.7 is the last version that worked reasonably well. I reported the bug in one of the 6.6.8 betas. At the time of 6.6.7 I was using vassal 3.7.5, 3.7.6 and 3.7.8, but I am just now running vasl 6.6.7 on vassal 3.7.14. vasl 6.6.7 has a limited quirk in that as the mouse cursor moves close to the edge, the drag image does not leave the boundaries of the current java window. The parts that are off the current java window are not visible either in non-vasl screen space nor on other vasl windows. As the mouse cursor gets closer to the edge more of the drag image disappears. If the cursor moves entirely out of vasl windows then the drag image is no longer shown. But once the mouse cursor enters another vasl window the drag image is suddenly drawn on this new vasl window, but again the image is cropped to the limits of the new vasl window. I like to use the hex grid draggable overlay for testing this because it is very big and you can see it get more and more cropped as it exits a window.

I am assuming this is because the drag-and-drop image drawing is done by getting mouse-move messages and drawing the drag image at that point inside java. The windowing system would crop any attempt to draw outside existing application windows. As I understand the process, in Microsoft Windows the "right" way to do drag-and-drop was to change the cursor to an image that included the dragged item. Then even when the cursor moved out of the application's window the drag image would "follow" the mouse around, although in reality the drag image was part of the cursor. I think I may have read that the cursor-change trick does not work outside of Microsoft Windows, but it's been a long time since I tried to learn drag-and-drop, so my recollection may be faulty. In vasl 6.6.10 drag-and-drop follows 6.6.7 when exiting a vasl window, i.e. the drag image gets cropped as the mouse cursor approaches the edge of the current window, but the drag image does not reappear when the mouse cursor enters a different vasl window.

Technically 6.6.7 was buggy in the sense that the drag image was drawn in at most one vasl window and would not draw outside the current vasl window, but it's probably not something I would have noticed because it does pop up on a different vasl window when the mouse enters that window. And because I use the combined application window the only time I would have experienced the vasl 6.6.7 was when dragging overlays. The vasl 6.6.10 behavior is that the drag image does not reappear once the mouse cursor enters a new vasl window.

This is very helpful. First of all, it tells us that the "disappearing" bug is a long-standing problem and not related to the other Linux bugs caused by the addition of code to support boardZoomer, starting in 6.6.8. Secondly, it gives us a before-after range (667 - 669) to look for causes.

Therefore, I think the following seems like the best course of action:

  1. Pull the 6610 changes into develop on GitHub.
  2. Release 6.6.10
  3. Close this issue
  4. Begin the 6.7.0 development cycle
  5. Create a new issue for the "disappearing drag image" and target its resolution for 6.7.0.

@geezer09 as we discussed, I suggest that I publish 6.6.10 as I have done in the past and that we use your workflow for the first 6.7.0 beta release.

derimmer commented 4 days ago

@geezer09 our posts overlapped but I think are consistent. No rush on the "disappearing" bug.