Bug 2816988: IllegalArgumentException in StackMetrics

I’ve been looking at this bug and found that the exception is thrown whenever
StackMetrics.relativePosition() is called on a peice which is not in the
Stack.

My question is: Does calling StackMetrics.relativePosition() on a piece not
in the Stack really indicate a bug, or should relativePosition() act more
like a “find” method, where it returns a designated value (like null) when
the piece doesn’t exist?

The relativePosition() method is used to decide where to draw a piece within a stack or to decide which piece within a stack the user has clicked on. If the code is trying to do this for a piece not in the stack, it definitely sounds like a bug. There shouldn’t be any places where the code calls this method to decide whether a piece belongs to a stack. That should be done by looking for the piece’s parent or enumerating the stack’s contents.

rk

Post generated using Mail2Forum (mail2forum.com)

Thus spake Rodney Kinney:

Do you want to work on this one? I’ve looked at it but can’t see why the
piece isn’t in the stack.


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Some more clues…

There is a check at Map:1601 that specifically tests that the piece in question has a non null parent and then calls StackMetrics.relativePosition() to find the position of that piece in the parent.

The Error message tells us that the piece is a counter called Delhi and it’s parent is Deck containing the single piece called Baghdad. Either the piece has been removed from the Deck by a different thread since Map:1601 (unlikely), or the piece has been removed from the Deck, but still thinks it belongs to it (more likely)

Another clue is that the command that is trying to update the piece has come from a different client accross the network. Bug 2763811 is the identical problem in a completely different module - Deck with one piece, networked ChangePiece command.

PS. An unrelated way to generate the same Exception is to set the ‘Maximum Cards to Display’ field in a Deck to 0.

PSS. A bandaid fix would be for relativePosition() to return 0 instead of throwing an exception.

B.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

I have a concern about the way we use SwingUtilities.invokeLater() to fire off additional threads all over the place with no sort of explicit synchonization whatsoever. Maybe I don’t know enought about SwingUtilites.invokeLater(), but my reading does not indicate the the threads you start via invokeLater() are guaranteed to run Atomically, only that they will be executed asynchronously after all pending AWT events have been processed.

Is it possible for a thread initiated by invokeLater() to suspend before it has completed and another to run, then the initial thread resumes?

B.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Thus spake “Brent Easton”:

I’m more than just concerned about it. I’m certain it’s wrong. This is one
of the things I’m planning to clean up in doing the model-view separation
in September.

Yes, that’s correct.

If invokeLater() is called on a Runnable from the EDT, then the already-
running code will complete before the Runnable starts. If invokeLater() is
called off the EDT, then you have no guarantees at all about when the
Runnable will run.


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Calling invokeLater() doesn’t start a new thread. It places work on the GUI thread, so it’s effectively a way of synchronizing events. You don’t have control over the exact order in which the events will be executed, but you guarantee that they won’t be interleaved.

This is not possible. All code in one invokeLater() call will complete before the code in the next invokeLater() is executed.

rk

Post generated using Mail2Forum (mail2forum.com)

That is reassuring.

B.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)