Skip to content

Commit

Permalink
feat: warn players when XACT-errors are in their logs (#3290)
Browse files Browse the repository at this point in the history
* feat: warn players when XACT-errors are in their logs

* add unit tests

* Fixed link to be clickable, added it as a button

* UX flow reworked

 * Fixed comments

* Rearrange buttons

* Make info icon bigger

* minor refactor

* refactor: remove record class
  • Loading branch information
Ivan-Shaml authored Jan 7, 2025
1 parent 01f2f89 commit 74c9618
Show file tree
Hide file tree
Showing 5 changed files with 192 additions and 13 deletions.
69 changes: 56 additions & 13 deletions src/main/java/com/faforever/client/game/GameRunner.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.faforever.client.i18n.I18n;
import com.faforever.client.leaderboard.LeaderboardService;
import com.faforever.client.logging.LoggingService;
import com.faforever.client.logging.analysis.LogAnalyzerService;
import com.faforever.client.main.event.ShowReplayEvent;
import com.faforever.client.map.MapService;
import com.faforever.client.mapstruct.GameMapper;
Expand Down Expand Up @@ -54,6 +55,9 @@
import javafx.beans.property.ReadOnlyObjectWrapper;
import javafx.beans.property.SimpleBooleanProperty;
import javafx.beans.property.SimpleObjectProperty;
import javafx.scene.Parent;
import javafx.scene.control.Button;
import javafx.scene.layout.Region;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.InitializingBean;
Expand All @@ -64,7 +68,9 @@
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
Expand Down Expand Up @@ -110,6 +116,7 @@ public class GameRunner implements InitializingBean {
private final NavigationHandler navigationHandler;
private final NotificationPrefs notificationPrefs;
private final FxApplicationThreadExecutor fxApplicationThreadExecutor;
private final LogAnalyzerService logAnalyzerService;

private final MaskPatternLayout logMasker = new MaskPatternLayout();
private final SimpleObjectProperty<Integer> runningGameId = new SimpleObjectProperty<>();
Expand Down Expand Up @@ -404,18 +411,22 @@ private void handleTermination(Process finishedProcess) {
fafServerAccessor.setTimeoutLoginReconnectSeconds(30);
int exitCode = finishedProcess.exitValue();
log.info("Forged Alliance terminated with exit code {}", exitCode);
Optional<Path> logFile = loggingService.getMostRecentGameLogFile();
logFile.ifPresent(file -> {
try {
Files.writeString(file, logMasker.maskMessage(Files.readString(file)));
} catch (IOException e) {
log.warn("Could not open log file", e);
}
});
Optional<Path> logFilePath = loggingService.getMostRecentGameLogFile();
Optional<String> logFileContent = logFilePath
.map(file -> {
try {
final String logFileText = logMasker.maskMessage(Files.readString(file));
Files.writeString(file, logFileText);
return logFileText;
} catch (IOException e) {
log.warn("Could not open log file", e);
return null;
}
});

if (!gameKilled) {
if (exitCode != 0) {
alertOnBadExit(exitCode, logFile);
alertOnBadExit(exitCode, logFilePath, logFileContent);
} else if (notificationPrefs.isAfterGameReviewEnabled()) {
askForGameRate();
}
Expand All @@ -433,21 +444,53 @@ private void askForGameRate() {
new Action(i18n.get("game.rate"), () -> navigationHandler.navigateTo(new ShowReplayEvent(game.getId()))))));
}

private void alertOnBadExit(int exitCode, Optional<Path> logFile) {
private void alertOnBadExit(int exitCode, Optional<Path> logFilePath, Optional<String> logFileContent) {
if (exitCode == -1073741515) {
notificationService.addImmediateWarnNotification("game.crash.notInitialized");
} else {
notificationService.addNotification(new ImmediateNotification(i18n.get("errorTitle"),
i18n.get("game.crash", exitCode,
logFile.map(Path::toString).orElse("")),
logFilePath.map(Path::toString).orElse("")),
WARN, List.of(new Action(i18n.get("game.open.log"),
() -> platformService.reveal(
logFile.orElse(
logFilePath.orElse(
operatingSystem.getLoggingDirectory()))),
new DismissAction(i18n))));
new DismissAction(i18n)),
getAnalysisButtonIfNecessary(logFileContent).orElse(null)));
}
}

private Optional<Parent> getAnalysisButtonIfNecessary(Optional<String> logFileContent) {
return logFileContent.map(content -> {
final Map<String, Action> analysisResult = logAnalyzerService.analyzeLogContents(content);
if (!analysisResult.isEmpty()) {
final StringBuilder message = new StringBuilder();
final List<Action> actions = new ArrayList<>();
analysisResult.forEach((msg, action) -> {
message.append(" - ").append(msg).append(System.lineSeparator());
if (action != null) {
actions.add(action);
}
});
actions.add(new DismissAction(i18n));

final Region infoIcon = new Region();
infoIcon.setId("btnInfoIcon");
infoIcon.getStyleClass().add("icon");
infoIcon.getStyleClass().add("icon24x24");
infoIcon.getStyleClass().add("info-icon");
final Button showAnalysisBtn = new Button(i18n.get("game.log.analysis.solutionBtn"), infoIcon);
showAnalysisBtn.setDefaultButton(true);
showAnalysisBtn.setOnAction(event -> notificationService.addNotification(
new ImmediateNotification(i18n.get("game.log.analysis"), message.toString(), WARN, actions)));

return showAnalysisBtn;
}
return null;
});
}


private void killGame() {
if (isRunning()) {
gameKilled = true;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package com.faforever.client.logging.analysis;

import com.faforever.client.config.ClientProperties;
import com.faforever.client.fx.PlatformService;
import com.faforever.client.i18n.I18n;
import com.faforever.client.notification.Action;
import lombok.RequiredArgsConstructor;
import org.apache.commons.lang3.StringUtils;
import org.jetbrains.annotations.NotNull;
import org.springframework.stereotype.Service;

import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

@Service
@RequiredArgsConstructor
public class LogAnalyzerService {
private static final String GAME_MINIMIZED_TRACE = "info: Minimized true";
private static final String SND_WARNING_TRACE = "warning: SND";
private static final String SND_XACT_TRACE = "XACT";

private final I18n i18n;
private final ClientProperties clientProperties;
private final PlatformService platformService;

@NotNull
public Map<String, Action> analyzeLogContents(final String logContents) {
final Map<String, Action> analysisResult = new HashMap<>();

if (StringUtils.contains(logContents, GAME_MINIMIZED_TRACE)) {
analysisResult.put(i18n.get("game.log.analysis.minimized"), null);
}

if (StringUtils.contains(logContents, SND_WARNING_TRACE) && StringUtils.contains(logContents, SND_XACT_TRACE)) {
final String moreInfoButtonCaption = i18n.get("game.log.analysis.moreInfoBtn");
final Action openSoundHelpAction = new Action(moreInfoButtonCaption, () -> platformService.showDocument(
clientProperties.getLinks().get("linksSoundIssues")));

analysisResult.put(i18n.get("game.log.analysis.snd", moreInfoButtonCaption), openSoundHelpAction);
}

return Collections.unmodifiableMap(analysisResult);
}
}
1 change: 1 addition & 0 deletions src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ faf-client:
patreon: https://www.patreon.com/faf
twitter: https://twitter.com/FAFOfficial_
wiki: https://wikijs.faforever.com/
linksSoundIssues: https://forum.faforever.com/topic/4084/solutions-for-snd-error-xact-invalid-arg-xact3dapply-failed

vault:
map_rules_url: https://wiki.faforever.com/en/Development/Vault/Rules
Expand Down
5 changes: 5 additions & 0 deletions src/main/resources/i18n/messages.properties
Original file line number Diff line number Diff line change
Expand Up @@ -1030,6 +1030,11 @@ getHelp = Get Help
copyError = Copy Error
findHelp = If this error continues use the Get Help button to open the help forum. Make sure to check the FAQ and previous topics. If you make a new topic please include your logs which can be opened from the menu in the top left of the client.
game.crash = It looks like Supreme Commander\: Forged Alliance crashed with exit code {0}. Check the game log {1} for more information.
game.log.analysis = The logs from your game have been automatically analyzed, revealing the following issue(s):
game.log.analysis.minimized = Minimizing the game or using Alt+Tab while it's in fullscreen mode may cause crashes. We recommend playing in windowed mode
game.log.analysis.snd = Certain operating system settings or third-party sound software can cause issues and game crashes. For more information, please click the ''{0}'' button
game.log.analysis.moreInfoBtn = More Information
game.log.analysis.solutionBtn = Show possible solutions
replayNotAvailable = Replay {0,number,#} is not yet available from the server. Please try again later.
close = Close
userMenu.moderationReport = Moderation reports
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package com.faforever.client.logging.analysis;

import com.faforever.client.config.ClientProperties;
import com.faforever.client.fx.PlatformService;
import com.faforever.client.i18n.I18n;
import com.faforever.client.notification.Action;
import com.faforever.client.test.ServiceTest;
import org.junit.jupiter.api.Test;
import org.mockito.InjectMocks;
import org.mockito.Mock;

import java.util.Map;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.when;

public class LogAnalyzerServiceTest extends ServiceTest {

private static final String SOUND_EXPECTED_TEXT = "Sound issue detected";
private static final String MINIMIZED_EXPECTED_TEXT = "Game was minimized";
private static final String MORE_INFO_BUTTON = "More Info";

@Mock
private I18n i18n;

@Mock
private ClientProperties clientProperties;

@Mock
private PlatformService platformService;

@InjectMocks
private LogAnalyzerService logAnalyzerService;

@Test
public void testAnalyzeLogContentsWhenGameMinimizedTrace() {
final String logContents = "info: Minimized true";

when(i18n.get("game.log.analysis.minimized")).thenReturn(MINIMIZED_EXPECTED_TEXT);

Map<String, Action> result = logAnalyzerService.analyzeLogContents(logContents);

assertTrue(result.containsKey(MINIMIZED_EXPECTED_TEXT));
}

@Test
public void testAnalyzeLogContentsWhenXactTrace() {
final String logContents = "warning: SND\nXACT";

when(i18n.get("game.log.analysis.moreInfoBtn")).thenReturn(MORE_INFO_BUTTON);
when(i18n.get("game.log.analysis.snd", MORE_INFO_BUTTON)).thenReturn(SOUND_EXPECTED_TEXT);

Map<String, Action> result = logAnalyzerService.analyzeLogContents(logContents);

assertEquals(1, result.size());
assertNotNull(result.get(SOUND_EXPECTED_TEXT));
}

@Test
public void testAnalyzeLogContentsWhenGameMinimizedAndXactTrace() {
final String logContents = "info: Minimized true\nwarning: SND\nXACT";

when(i18n.get("game.log.analysis.minimized")).thenReturn(MINIMIZED_EXPECTED_TEXT);
when(i18n.get("game.log.analysis.moreInfoBtn")).thenReturn(MORE_INFO_BUTTON);
when(i18n.get("game.log.analysis.snd", MORE_INFO_BUTTON)).thenReturn(SOUND_EXPECTED_TEXT);

Map<String, Action> result = logAnalyzerService.analyzeLogContents(logContents);

assertEquals(2, result.size());
assertNotNull(result.get(SOUND_EXPECTED_TEXT));
assertNull(result.get(MINIMIZED_EXPECTED_TEXT));
}

@Test
public void testAnalyzeLogContentsWhenNoRelevantTraces() {
final String logContents = "Some other log content";

Map<String, Action> result = logAnalyzerService.analyzeLogContents(logContents);

assertTrue(result.isEmpty());
}
}

0 comments on commit 74c9618

Please sign in to comment.