Skip to content

Commit

Permalink
Merge pull request #5092 from kwvanderlinde/fixup/5091-npe-while-dese…
Browse files Browse the repository at this point in the history
…rializing-campaign-with-no-walls

Various fixes for wall topology
  • Loading branch information
cwisniew authored Dec 7, 2024
2 parents ede05a0 + a6921c5 commit 763510d
Show file tree
Hide file tree
Showing 4 changed files with 159 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,13 @@
package net.rptools.maptool.model.converters;

import com.thoughtworks.xstream.XStream;
import com.thoughtworks.xstream.annotations.XStreamAliasType;
import com.thoughtworks.xstream.annotations.XStreamAsAttribute;
import com.thoughtworks.xstream.annotations.XStreamImplicit;
import com.thoughtworks.xstream.converters.MarshallingContext;
import com.thoughtworks.xstream.converters.UnmarshallingContext;
import com.thoughtworks.xstream.converters.collections.AbstractCollectionConverter;
import com.thoughtworks.xstream.io.HierarchicalStreamReader;
import com.thoughtworks.xstream.io.HierarchicalStreamWriter;
import java.awt.geom.Point2D;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
Expand Down Expand Up @@ -81,12 +80,12 @@ public void marshal(Object source, HierarchicalStreamWriter writer, MarshallingC
vertex ->
new VertexRepresentation(
vertex.id(), vertex.getPosition().getX(), vertex.getPosition().getY()))
.forEach(representation.vertices.list::add);
.forEach(representation.vertices::add);
walls
.getWalls()
.sorted(Comparator.comparingInt(WallTopology.Wall::getZIndex))
.map(wall -> new WallRepresentation(wall.from().id(), wall.to().id()))
.forEach(representation.walls.list::add);
.forEach(representation.walls::add);

context.convertAnother(representation);
}
Expand All @@ -96,7 +95,7 @@ public Object unmarshal(HierarchicalStreamReader reader, UnmarshallingContext co
var walls = new WallTopology();
var representation =
(GraphRepresentation) context.convertAnother(walls, GraphRepresentation.class);
for (var vertexRepresentation : representation.vertices.list) {
for (var vertexRepresentation : representation.vertices) {
try {
var vertex = walls.createVertex(new GUID(vertexRepresentation.id));
vertex.setPosition(vertexRepresentation.x, vertexRepresentation.y);
Expand All @@ -107,7 +106,7 @@ public Object unmarshal(HierarchicalStreamReader reader, UnmarshallingContext co
e);
}
}
for (var wallRepresentation : representation.walls.list) {
for (var wallRepresentation : representation.walls) {
try {
walls.createWall(new GUID(wallRepresentation.from), new GUID(wallRepresentation.to));
} catch (WallTopology.GraphException e) {
Expand All @@ -118,26 +117,20 @@ public Object unmarshal(HierarchicalStreamReader reader, UnmarshallingContext co
e);
}
}

walls.removeDanglingVertices();

return walls;
}

// region Intermediate representation that contains only the essence of the wall graph.

private static final class GraphRepresentation {
public final VerticesRepresentation vertices = new VerticesRepresentation();
public final WallsRepresentation walls = new WallsRepresentation();
}

private static final class VerticesRepresentation {
@XStreamImplicit(itemFieldName = "vertex")
public final List<VertexRepresentation> list = new ArrayList<>();
}

private static final class WallsRepresentation {
@XStreamImplicit(itemFieldName = "wall")
public final List<WallRepresentation> list = new ArrayList<>();
public final List<VertexRepresentation> vertices = new ArrayList<>();
public final List<WallRepresentation> walls = new ArrayList<>();
}

@XStreamAliasType("vertex")
private static final class VertexRepresentation {
@XStreamAsAttribute public final String id;
@XStreamAsAttribute public final double x;
Expand All @@ -150,6 +143,7 @@ public VertexRepresentation(GUID id, double x, double y) {
}
}

@XStreamAliasType("wall")
private static final class WallRepresentation {
@XStreamAsAttribute public final String from;
@XStreamAsAttribute public final String to;
Expand All @@ -160,15 +154,5 @@ public WallRepresentation(GUID from, GUID to) {
}
}

private static final class PointRepresentation {
@XStreamAsAttribute public final double x;
@XStreamAsAttribute public final double y;

public PointRepresentation(Point2D position) {
this.x = position.getX();
this.y = position.getY();
}
}

// endregion
}
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,14 @@ public Vertex merge(Vertex first, Vertex second) {

// endregion

private void removeDanglingVertices() {
/**
* Ensures that any vertices with no walls are removed from the graph.
*
* <p>This is usually called automatically when needed, so don't call this method without reason.
* Deserializing is an exception where improper XML could result in vertices being added without
* associated walls.
*/
public void removeDanglingVertices() {
var verticesToRemove = new ArrayList<GUID>();
for (var vertex : graph.vertexSet()) {
if (graph.edgesOf(vertex).isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2771,6 +2771,8 @@ tool.isorectangletopology.instructions = LClick: set initial/final point, Shif
tool.stamp.tooltip = Stamp tool
tool.walltemplate.instructions = LClick: set starting cell, move mouse in direction of wall, second LClick to finish wall; Ctrl: move origin point
tool.walltemplate.tooltip = Draw a wall template.
tool.walltopology.tooltip = Draw movement and vision blocking walls.
tool.walltopology.instructions = LClick: start new wall or grab wall / vertex, RClick: extend wall, Alt+LClick: extend or split wall / vertex, Alt+Drag: join with wall / vertex, Ctrl: snap-to-grid, Shift: delete wall / vertex
tools.ai_selector.tooltip = Tokens will navigate around MBL, VBL (if selected), and account for tokens with terrain modifiers.
tools.ignore_vbl_on_move.tooltip = Tokens will navigate around VBL if selected, otherwise they will freely move into VBL.
tools.topology_mode_selection.cover_vbl.tooltip = Draw Cover Vision Blocking (Cover VB).
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
/*
* This software Copyright by the RPTools.net development team, and
* licensed under the Affero GPL Version 3 or, at your option, any later
* version.
*
* MapTool Source Code is distributed in the hope that it will be
* useful, but WITHOUT ANY WARRANTY; without even the implied warranty
* of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
*
* You should have received a copy of the GNU Affero General Public
* License * along with this source Code. If not, please visit
* <http://www.gnu.org/licenses/> and specifically the Affero license
* text at <http://www.gnu.org/licenses/agpl.html>.
*/
package net.rptools.maptool.model.converters;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Named.named;
import static org.junit.jupiter.params.provider.Arguments.arguments;

import com.thoughtworks.xstream.XStream;
import java.awt.geom.Point2D;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import net.rptools.maptool.model.topology.WallTopology;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

/**
* Tests that {@link WallTopologyConverter} can be used correctly with {@link XStream} for
* serializing and deserializing.
*/
public class WallTopologyConverterTest {
private XStream xStream;

@BeforeEach
void setUp() {
xStream = new XStream();
XStream.setupDefaultSecurity(xStream);
xStream.allowTypesByWildcard(new String[] {"net.rptools.**", "java.awt.**", "sun.awt.**"});
xStream.registerConverter(new WallTopologyConverter(xStream));
}

@ParameterizedTest
@MethodSource
void testSerializeDeserialize(WallTopology topology) {
var xml = xStream.toXML(topology);
var result = xStream.fromXML(xml);
assertInstanceOf(WallTopology.class, result);
var newTopology = (WallTopology) result;

var originalVertices = topology.getVertices().collect(Collectors.toSet());
var originalWalls = topology.getWalls().collect(Collectors.toSet());
var newVertices = newTopology.getVertices().collect(Collectors.toSet());
var newWalls = newTopology.getWalls().collect(Collectors.toSet());

assertEquals(originalVertices, newVertices);
assertEquals(originalWalls, newWalls);
}

static List<Arguments> testSerializeDeserialize() {
var result = new ArrayList<Arguments>();

{
var topology = new WallTopology();
result.add(arguments(named("empty walls", topology)));
}
{
var topology = new WallTopology();
topology.brandNewWall();
result.add(arguments(named("single default wall", topology)));
}
{
var topology = new WallTopology();
topology.string(
new Point2D.Double(0, 0),
builder -> {
builder.push(new Point2D.Double(100, 0));
builder.push(new Point2D.Double(100, 100));
builder.push(new Point2D.Double(50, 0));
});
result.add(arguments(named("wall string", topology)));
}

return result;
}

@Test
void testDeserializeEmptyVerticesAndWalls() {
var xml =
"""
<net.rptools.maptool.model.topology.WallTopology>
<vertices />
<walls />
</net.rptools.maptool.model.topology.WallTopology>
""";

var result = xStream.fromXML(xml);
assertInstanceOf(WallTopology.class, result);
var newTopology = (WallTopology) result;

assertEquals(Set.of(), newTopology.getVertices().collect(Collectors.toSet()));
assertEquals(Set.of(), newTopology.getWalls().collect(Collectors.toSet()));
}

@Test
void testDeserializeEmptyWalls() {
// This is not something that is supposed to appear in the XML, but we might as well handle it.
// There are no walls, meaning the vertices are all dangling and shouldn't be there either.
// This also tests that the implicit collection for walls is safe to deserialize when empty.

var xml =
"""
<net.rptools.maptool.model.topology.WallTopology>
<vertices>
<vertex id="549C67BA47264D52B45AE9F56189697A" x="0.0" y="0.0"/>
<vertex id="72E33F5BEBDE4151B94A1EFD3C9599A1" x="1000.0" y="0.0"/>
</vertices>
<walls />
</net.rptools.maptool.model.topology.WallTopology>
""";

var result = xStream.fromXML(xml);
assertInstanceOf(WallTopology.class, result);
var newTopology = (WallTopology) result;

// The vertices in the XML are dangling, so are automatically removed.
assertEquals(Set.of(), newTopology.getVertices().collect(Collectors.toSet()));
assertEquals(Set.of(), newTopology.getWalls().collect(Collectors.toSet()));
}
}

0 comments on commit 763510d

Please sign in to comment.