Add error if multiplayer tag is defined for nonexistent track (#54)

Nianna · Oct 7, 2024 · ca5e2fc · ca5e2fc
1 parent 462657b
commit ca5e2fc
Show file tree

Hide file tree

Showing 7 changed files with 50 additions and 24 deletions.
diff --git a/src/main/java/com/github/nianna/karedi/problem/TagForNonexistentTrackProblem.java b/src/main/java/com/github/nianna/karedi/problem/TagForNonexistentTrackProblem.java
@@ -0,0 +1,11 @@
+package com.github.nianna.karedi.problem;
+
+import com.github.nianna.karedi.I18N;
+
+public class TagForNonexistentTrackProblem extends TagProblem {
+
+    public TagForNonexistentTrackProblem(String key) {
+        super(Severity.WARNING, I18N.get("problem.tag.nonexistent_track.title", key), key);
+    }
+
+}
diff --git a/src/main/java/com/github/nianna/karedi/song/SongChecker.java b/src/main/java/com/github/nianna/karedi/song/SongChecker.java
@@ -10,7 +10,9 @@
 import com.github.nianna.karedi.problem.Problem.Severity;
 import com.github.nianna.karedi.problem.Problematic;
 import com.github.nianna.karedi.problem.ProblemsCombiner;
+import com.github.nianna.karedi.problem.TagForNonexistentTrackProblem;
 import com.github.nianna.karedi.problem.TagValidationErrorProblem;
+import com.github.nianna.karedi.song.tag.MultiplayerTags;
 import com.github.nianna.karedi.song.tag.Tag;
 import com.github.nianna.karedi.song.tag.TagKey;
 import com.github.nianna.karedi.song.tag.TagKeyValidator;
@@ -33,7 +35,7 @@ public SongChecker(Song song) {
 		this.song = song;
 		combiner = new ProblemsCombiner(song.getTracks());
 		song.getTracks().addListener(ListenersUtils.createListChangeListener(this::refreshTrack,
-				this::refreshTrack, this::refreshTrack, this::onRemoved));
+				this::refreshTrack, this::onTrackAdded, this::onTrackRemoved));
 		song.getTags().addListener(ListenersUtils.createListChangeListener(ListenersUtils::pass,
 				this::refreshTag, this::refreshTag, this::onTagRemoved));
 
@@ -46,14 +48,19 @@ public SongChecker(Song song) {
 		song.upperXBoundProperty().addListener(obs -> checkEnd());
 	}
 
-	private void onRemoved(SongTrack track) {
+	private void onTrackRemoved(SongTrack track) {
 		removeTrackProblems(track);
 		if (song.getTrackCount() > 0) {
 			refreshTrack(song.getTrack(0));
+			revalidateAllMultiplayerTags();
 		} else {
 			// should never happen
 		}
+	}
 
+	private void onTrackAdded(SongTrack track) {
+		refreshTrack(track);
+		revalidateAllMultiplayerTags();
 	}
 
 	private void removeTrackProblems(SongTrack track) {
@@ -100,6 +107,11 @@ private void validateTagKey(String tagKey) {
 		song.formatSpecificationVersion()
 				.flatMap(formatVersion -> TagKeyValidator.validate(tagKey, formatVersion))
 				.ifPresent(combiner::add);
+		if (MultiplayerTags.isANameTagKey(tagKey)) {
+			MultiplayerTags.getTrackNumber(tagKey)
+					.filter(trackIndex -> trackIndex >= song.getTrackCount())
+					.ifPresent(ignored -> combiner.add(new TagForNonexistentTrackProblem(tagKey)));
+		}
 
 	}
 
@@ -127,6 +139,12 @@ private void revalidateAllTagsOnVersionChange() {
 				.forEach(this::refreshTag);
 	}
 
+	private void revalidateAllMultiplayerTags() {
+		song.getTags().stream()
+				.filter(MultiplayerTags::isANameTag)
+				.forEach(this::refreshTag);
+	}
+
 	private void validateValue(TagKey key, String value) {
 		ValidationResult result = TagValueValidators.validate(key, value);
 		if (!result.getErrors().isEmpty()) {

diff --git a/src/main/java/com/github/nianna/karedi/song/tag/MultiplayerTags.java b/src/main/java/com/github/nianna/karedi/song/tag/MultiplayerTags.java
@@ -15,7 +15,11 @@ private MultiplayerTags() {
 	}
 
 	public static boolean isANameTag(Tag tag) {
-		return MULTIPLAYER_TAG_NAME_PATTERN.matcher(tag.getKey()).matches();
+		return isANameTagKey(tag.getKey());
+	}
+
+	public static boolean isANameTagKey(String tagKey) {
+		return MULTIPLAYER_TAG_NAME_PATTERN.matcher(tagKey).matches();
 	}
 
 	public static String getTagKeyForTrackNumber(int index, FormatSpecification formatSpecification) {
@@ -27,11 +31,15 @@ public static String getTagKeyForTrackNumber(int index, FormatSpecification form
 	}
 
 	public static Optional<Integer> getTrackNumber(Tag tag) {
-		return getPlayerNumber(tag).map(player -> player - 1);
+		return getTrackNumber(tag.getKey());
+	}
+
+	public static Optional<Integer> getTrackNumber(String tagKey) {
+		return getPlayerNumber(tagKey).map(player -> player - 1);
 	}
 
-	public static Optional<Integer> getPlayerNumber(Tag tag) {
-		Matcher matcher = MULTIPLAYER_TAG_NAME_PATTERN.matcher(tag.getKey());
+	private static Optional<Integer> getPlayerNumber(String tagKey) {
+		Matcher matcher = MULTIPLAYER_TAG_NAME_PATTERN.matcher(tagKey);
 		if (matcher.find()) {
 			return Converter.toInteger(matcher.group(2));
 		}

diff --git a/src/main/resources/messages.properties b/src/main/resources/messages.properties
@@ -349,6 +349,7 @@ problem.line.overlapping.description=Lines should not overlap
 problem.tag.validation_fail.title=Tag validation failed
 problem.tag.validation_fail.full_title=Tag validation failed #{0}
 problem.tag.unsupported.title=Unsupported tag #{0}
+problem.tag.nonexistent_track.title=Tag #{0} defines name for nonexistent track
 problem.tag.inconsistent.title=Tags #{0} and #{1} are inconsistent
 problem.tag.inconsistent.description=These tags have the same meaning, remove one of them or use the same value
 problem.uncommon_golden_bonus.title=Uncommon golden bonus

diff --git a/src/main/resources/messages_en_GB.properties b/src/main/resources/messages_en_GB.properties
@@ -349,6 +349,7 @@ problem.line.overlapping.description=Lines should not overlap
 problem.tag.validation_fail.title=Tag validation failed
 problem.tag.validation_fail.full_title=Tag validation failed #{0}
 problem.tag.unsupported.title=Unsupported tag #{0}
+problem.tag.nonexistent_track.title=Tag #{0} defines name for nonexistent track
 problem.tag.inconsistent.title=Tags #{0} and #{1} are inconsistent
 problem.tag.inconsistent.description=These tags have the same meaning, remove one of them or use the same value
 problem.uncommon_golden_bonus.title=Uncommon golden bonus

diff --git a/src/main/resources/messages_pl_PL.properties b/src/main/resources/messages_pl_PL.properties
@@ -359,6 +359,7 @@ problem.tag.validation_fail.full_title=Niepoprawna warto\u015B\u0107 tagu #{0}
 problem.tag.inconsistent.title=Tagi #{0} i #{1} s\u0105 niespójne
 problem.tag.inconsistent.description=Te tagi maj\u0105 t\u0105 sam\u0105 funkcj\u0119, usu\u0144 jeden z nich lub ustaw t\u0105 sam\u0105 warto\u015B\u0107
 problem.tag.unsupported.title=Niewspierany tag #{0}
+problem.tag.nonexistent_track.title=Tag #{0} definiuje nazw\u0119 dla nieistniej\u0105cej \u015Bcie\u017Cki
 problem.uncommon_golden_bonus.title=Niepoprawny bonus za z\u0142ote nuty
 problem.uncommon_golden_bonus.full_title=Niepoprawny bonus za z\u0142ote nuty ({0, number} punktów)
 problem.uncommon_golden_bonus.description=Z\u0142ote nuty powinny by\u0107 warte od {0, number, integer} do {1, number, integer} punktów

diff --git a/src/test/java/com/github/nianna/karedi/song/tag/MultiplayerTagsTest.java b/src/test/java/com/github/nianna/karedi/song/tag/MultiplayerTagsTest.java
@@ -29,30 +29,16 @@ public void otherTagKeysAreNotRecognizedAsNameTags() {
 	}
 
 	@Test
-	public void extractsCorrectPlayerNumbersFromValidDuetSingerPNameTags() {
-		Tag tag = new Tag("DUETSINGERP2", "");
-		Optional<Integer> result = MultiplayerTags.getPlayerNumber(tag);
+	public void extractsCorrectTrackNumbersFromValidDuetSingerPTags() {
+		Tag tag = new Tag("DUETSINGERP3", "");
+		Optional<Integer> result = MultiplayerTags.getTrackNumber(tag);
 		assertTrue(result.isPresent());
 		assertEquals((Integer) 2, result.get());
 	}
 
 	@Test
-	public void extractsCorrectPlayerNumbersFromValidPNameTags() {
+	public void extractsCorrectTrackNumbersFromValidPTags() {
 		Tag tag = new Tag("P3", "");
-		Optional<Integer> result = MultiplayerTags.getPlayerNumber(tag);
-		assertTrue(result.isPresent());
-		assertEquals((Integer) 3, result.get());
-	}
-
-	@Test
-	public void doesNotExtractPlayerNumbersFromOtherTags() {
-		Tag tag = new Tag("ARTIST", "");
-		assertFalse(MultiplayerTags.getPlayerNumber(tag).isPresent());
-	}
-
-	@Test
-	public void extractsCorrectTrackNumbersFromValidNameTags() {
-		Tag tag = new Tag("DUETSINGERP3", "");
 		Optional<Integer> result = MultiplayerTags.getTrackNumber(tag);
 		assertTrue(result.isPresent());
 		assertEquals((Integer) 2, result.get());