deathindicators: Fix saving bones to config (#1896)

* Don't use a permanently-empty map when adding bones

* Save bones when adding them, rather than on shutdown/region change

runelite-client/src/main/java/net/runelite/client/plugins/deathindicator/Bones.java

@@ -3,7 +3,6 @@ package net.runelite.client.plugins.deathindicator;
 import com.google.common.collect.ImmutableMap;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -22,7 +21,6 @@ public class Bones
 	private static final String BONES_PREFIX = "bones_";
 
 	private ImmutableMap<Integer, Map<WorldPoint, List<Bone>>> map;
-	private boolean changed = false;
 
 	void init(Client client, ConfigManager configManager)
 	{
@@ -31,14 +29,14 @@ public class Bones
 		Bone[][] bones = getBones(configManager, regions);
 		if (log.isDebugEnabled())
 		{
-			log.debug("Regions are now {}", Arrays.toString(regions));
+			log.trace("Regions are now {}", Arrays.toString(regions));
 
 			int n = 0;
 			for (Bone[] ar : bones)
 			{
 				n += ar.length;
 			}
-				log.debug("Loaded {} Bones", n);
+			log.debug("Loaded {} Bones", n);
 		}
 
 		initMap(regions, bones);
@@ -81,7 +79,7 @@ public class Bones
 			Bone[] boneA = bones[i];
 			if (boneA.length == 0)
 			{
-				builder.put(region, Collections.EMPTY_MAP);
+				builder.put(region, new HashMap<>());
 				continue;
 			}
 
@@ -103,35 +101,29 @@ public class Bones
 		this.forEach(bone -> bone.addToScene(scene));
 	}
 
-	void save(ConfigManager configManager)
+	void save(ConfigManager configManager, int region)
 	{
-		if (this.map == null || !changed)
+		final Map<WorldPoint, List<Bone>> regionBones = this.map.get(region);
+		if (regionBones == null)
 		{
-			this.changed = false;
 			return;
 		}
 
-		for (Map.Entry<Integer, Map<WorldPoint, List<Bone>>> entry : this.map.entrySet())
+		final String key = BONES_PREFIX + region;
+
+		if (regionBones.size() == 0)
 		{
-			final String key = BONES_PREFIX + entry.getKey();
+			configManager.unsetConfiguration(CONFIG_GROUP, key);
-			final Map<WorldPoint, List<Bone>> map = entry.getValue();
-			if (map.size() == 0)
-			{
-				configManager.unsetConfiguration(CONFIG_GROUP, key);
-				continue;
-			}
-
-			List<Bone> list = new ArrayList<>(map.values().size());
-			for (List<Bone> lb : map.values())
-			{
-				list.addAll(lb);
-			}
-
-			String val = GSON.toJson(list.toArray(new Bone[0]));
-			configManager.setConfiguration(CONFIG_GROUP, key, val);
 		}
 
-		this.changed = false;
+		List<Bone> list = new ArrayList<>(regionBones.values().size());
+		for (List<Bone> lb : regionBones.values())
+		{
+			list.addAll(lb);
+		}
+
+		String val = GSON.toJson(list.toArray(new Bone[0]));
+		configManager.setConfiguration(CONFIG_GROUP, key, val);
 	}
 
 	public boolean add(Bone bone)
@@ -141,7 +133,6 @@ public class Bones
 			return false;
 		}
 
-		this.changed = true;
 		final int region = bone.getLoc().getRegionID();
 		final Map<WorldPoint, List<Bone>> map = this.map.get(region);
 		final List<Bone> list = map.computeIfAbsent(bone.getLoc(), wp -> new ArrayList<>());
@@ -151,7 +142,6 @@ public class Bones
 
 	public void remove(Bone bone)
 	{
-		this.changed = true;
 		final int region = bone.getLoc().getRegionID();
 		final Map<WorldPoint, List<Bone>> map = this.map.get(region);
 		final List<Bone> list = map.get(bone.getLoc());

runelite-client/src/main/java/net/runelite/client/plugins/deathindicator/DeathIndicatorPlugin.java

@@ -182,7 +182,6 @@ public class DeathIndicatorPlugin extends Plugin
 		worldMapPointManager.removeIf(DeathWorldMapPoint.class::isInstance);
 
 		clientThread.invokeLater(this::clearBones);
-		saveBones();
 	}
 
 	private void initBones()
@@ -190,11 +189,6 @@ public class DeathIndicatorPlugin extends Plugin
 		bones.init(client, configManager);
 	}
 
-	private void saveBones()
-	{
-		bones.save(configManager);
-	}
-
 	private void clearBones()
 	{
 		bones.clear(client.getScene());
@@ -236,12 +230,16 @@ public class DeathIndicatorPlugin extends Plugin
 
 	private void onPlayerDeath(PlayerDeath death)
 	{
-		Player p = death.getPlayer();
+		newBoneFor(death.getPlayer());
+	}
+
+	private void newBoneFor(Player player)
+	{
 		Bone b = new Bone();
 
-		b.setName(Text.sanitize(p.getName()));
+		b.setName(Text.sanitize(player.getName()));
 		b.setTime(Instant.now());
-		b.setLoc(p.getWorldLocation());
+		b.setLoc(player.getWorldLocation());
 
 		while (!bones.add(b))
 		{
@@ -249,6 +247,7 @@ public class DeathIndicatorPlugin extends Plugin
 		}
 
 		b.addToScene(client.getScene());
+		bones.save(configManager, b.getLoc().getRegionID());
 	}
 
 	private void onMenuEntryAdded(MenuEntryAdded event)
@@ -309,7 +308,13 @@ public class DeathIndicatorPlugin extends Plugin
 			return;
 		}
 
-		lastDeath = client.getLocalPlayer().getWorldLocation();
+		Player lp = client.getLocalPlayer();
+		if (config.permaBones())
+		{
+			newBoneFor(lp);
+		}
+
+		lastDeath = lp.getWorldLocation();
 		lastDeathWorld = client.getWorld();
 		lastDeathTime = Instant.now();
 	}
@@ -400,7 +405,6 @@ public class DeathIndicatorPlugin extends Plugin
 					if (client.getGameState() == GameState.LOGGED_IN)
 					{
 						clientThread.invokeLater(this::clearBones);
-						saveBones();
 					}
 				}
 				return;
@@ -438,7 +442,6 @@ public class DeathIndicatorPlugin extends Plugin
 		{
 			case LOADING:
 				clearBones();
-				saveBones();
 				break;
 			case LOGGED_IN:
 				if (config.permaBones())