Bug in latest build: incomplete deal

Technical Support & Bugs
1

Module: on google drive
Java:
openjdk 11.0.7 2020-04-14
OpenJDK Runtime Environment (build 11.0.7+10-post-Debian-3deb10u1)
OpenJDK 64-Bit Server VM (build 11.0.7+10-post-Debian-3deb10u1, mixed mode, sharing)
OS: Linux leonhartsberger 4.19.0-8-amd64 #1 SMP Debian 4.19.98-1+deb10u1 (2020-04-27) x86_64 GNU/Linux

Everything is fine in 3.2.17 (where this was last saved). Everything is fine in 3.3.0. When I try to play a game in the build I did in this thread: pdf doc I hit an issue:

It should look like this: 6 role cards dealt.
In the faulty version it looks like: only 4 cards dealt. (It makes no difference whether I bother to set the names or not.)

Steps to reproduce:

  1. Log on as the “Convener”.
  2. Enter the window marked “Role” from the toolbar.
  3. Click on the “bell” button in the top-left corner.
2

Not surprisingly if I just take a clean “git clone”. and build it still happens. So it was not the xdg-open fix that caused the problem. I did not think it would be but I am trying to see if I can pinpoint which commit did it.

3

I did a git bisect and I got the following:

[code]nicholas@leonhartsberger:~/VASSAL/src/vassal$ git bisect good
f6053d0c7f7ce28a4998282e5ac585f89c170cff is the first bad commit
commit f6053d0c7f7ce28a4998282e5ac585f89c170cff
Author: yanlyub yanlyub@users.noreply.github.com
Date: Mon Jun 15 21:46:28 2020 +0200

rewrote Stack.asList, removed outdated unit test

:040000 040000 a3d9dbd43215ccc96543aa5869f9144adafd8558 d6b060daaf71e0107c0bbdb08b2af57a23735656 M src
:040000 040000 e90421ace3bcb2d216cc46ff9f87c7a57949db8f 5576b564b9fa2d41f65e95d0a2ed454432e0b9e6 M test
nicholas@leonhartsberger:~/VASSAL/src/vassal$
[/code]

4

It looks like the offending code is here:

[code]— a/src/VASSAL/counters/Stack.java
+++ b/src/VASSAL/counters/Stack.java
@@ -90,11 +90,7 @@ public class Stack implements GamePiece, StateMergeable {
* @return an unmodifiable {@link List} of {@link GamePiece}s contained in th
is {@link Stack}
*/
public List asList() {

  • List result = new ArrayList<>();
  • for (int i = 0; i < pieceCount; i++) {
  •  result.add(contents[i]);
  • }
  • return Collections.unmodifiableList(result);
  • return Collections.unmodifiableList(Arrays.asList(contents).subList(0, pieceCount));
    }

[/code]

5

I found this also affects Tigris and Euphrates. In determining player order only 3 out of 4 tokens are dealt.

6

Good job with the bisect.

Before that commit, a completely new list was created, and elements inserted into it.

After the commit, Arrays.asList() creates a list BACKED BY the array, and .subList() creates a sub list BACKED BY the list, so any change to the original array ends up in the supposedly unmodifiable list. This can lead to various problems down the line.

7

A unit test confirmed my assumption, the List that is returned from asList() changes if pieces are removed from the stack after getting the list.

github.com/vassalengine/vassal/pull/29 should fix this by reverting it to the previous version with the manually built new list.

Some day we will get rid of this array extremism inside the Stack, with the manual counting of array contents and manual resizing of the array. For this we need to un-inherit the Deck from the Stack, then replace the array with a regular list. And write lots of unit tests. I’m on it.

8

Okay I did a fresh clone and pulled both fixes. Everything seems okay at the moment. Thanks.

9

Thus spake slimy:

Okay I did a fresh clone and pulled both fixes. Everything seems okay at
the moment. Thanks.

Fix is merged to master at dc947b5ab.

–
J.