From 2002af3e7130e575457936e96d043a9bc00a8056 Mon Sep 17 00:00:00 2001 From: iHDeveloper <20463031+iHDeveloper@users.noreply.github.com> Date: Mon, 30 Nov 2020 15:33:57 +0300 Subject: [PATCH] feat(discord): rewrite + make it thread-safe The rewrite was necessary to be able to achieve the system to be thread-safe. It gave the opportunity to organise how the subsystem works. This also ensures the subsystem is easy to: - Implement new features! - Fix bugs without breaking stuff - Separated the thread into its own class - Subsystem manages the communication with the thread - Re-organized the system - Fix: disable discord-ipc library logger --- facades/PC/src/main/resources/logback.xml | 1 + .../discordrpc/DiscordRPCBuffer.java | 71 +++++ .../discordrpc/DiscordRPCSubSystem.java | 296 ++++-------------- .../discordrpc/DiscordRPCSystem.java | 83 +++-- .../discordrpc/DiscordRPCThread.java | 250 +++++++++++++++ 5 files changed, 415 insertions(+), 286 deletions(-) create mode 100644 subsystems/DiscordRPC/src/main/java/org/terasology/engine/subsystem/discordrpc/DiscordRPCBuffer.java create mode 100644 subsystems/DiscordRPC/src/main/java/org/terasology/engine/subsystem/discordrpc/DiscordRPCThread.java diff --git a/facades/PC/src/main/resources/logback.xml b/facades/PC/src/main/resources/logback.xml index 10fe1541d74..7b4252d833d 100644 --- a/facades/PC/src/main/resources/logback.xml +++ b/facades/PC/src/main/resources/logback.xml @@ -50,4 +50,5 @@ + diff --git a/subsystems/DiscordRPC/src/main/java/org/terasology/engine/subsystem/discordrpc/DiscordRPCBuffer.java b/subsystems/DiscordRPC/src/main/java/org/terasology/engine/subsystem/discordrpc/DiscordRPCBuffer.java new file mode 100644 index 00000000000..c428fae860b --- /dev/null +++ b/subsystems/DiscordRPC/src/main/java/org/terasology/engine/subsystem/discordrpc/DiscordRPCBuffer.java @@ -0,0 +1,71 @@ +// Copyright 2020 The Terasology Foundation +// SPDX-License-Identifier: Apache-2.0 + +package org.terasology.engine.subsystem.discordrpc; + +import java.time.OffsetDateTime; + +/** + * A threaded-safe shared buffer used to store information for {@link DiscordRPCThread} to be processed as {@link com.jagrosh.discordipc.entities.RichPresence} + * + * It helps avoiding allocating unnecessary objects for the rich presence. + */ +public final class DiscordRPCBuffer { + private String state; + private OffsetDateTime startTimestamp; + private boolean changed; + + /** + * Sets the current party status + * + * @param state The current party status + */ + public synchronized void setState(String state) { + this.state = state; + this.changed = true; + } + + /** + * Returns the current party status + * + * @return The current party status + */ + public synchronized String getState() { + return state; + } + + /** + * Sets the start of the game + * + * @param startTimestamp The time when that action has start or null to hide it + */ + public synchronized void setStartTimestamp(OffsetDateTime startTimestamp) { + this.startTimestamp = startTimestamp; + this.changed = true; + } + + /** + * Returns the start of the game + * + * @return The start of the game + */ + public synchronized OffsetDateTime getStartTimestamp() { + return startTimestamp; + } + + /** + * Check if the buffer has changed + * + * @return if the buffer has changed + */ + public synchronized boolean hasChanged() { + return changed; + } + + /** + * Resets the buffer's change state to false + */ + synchronized void resetState() { + this.changed = false; + } +} diff --git a/subsystems/DiscordRPC/src/main/java/org/terasology/engine/subsystem/discordrpc/DiscordRPCSubSystem.java b/subsystems/DiscordRPC/src/main/java/org/terasology/engine/subsystem/discordrpc/DiscordRPCSubSystem.java index 18a4ad9449a..aa09123700c 100644 --- a/subsystems/DiscordRPC/src/main/java/org/terasology/engine/subsystem/discordrpc/DiscordRPCSubSystem.java +++ b/subsystems/DiscordRPC/src/main/java/org/terasology/engine/subsystem/discordrpc/DiscordRPCSubSystem.java @@ -15,13 +15,6 @@ */ package org.terasology.engine.subsystem.discordrpc; -import ch.qos.logback.classic.Level; -import ch.qos.logback.classic.LoggerContext; -import com.jagrosh.discordipc.IPCClient; -import com.jagrosh.discordipc.IPCListener; -import com.jagrosh.discordipc.entities.RichPresence; -import com.jagrosh.discordipc.entities.pipe.Pipe; -import com.jagrosh.discordipc.entities.pipe.WindowsPipe; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.terasology.config.Config; @@ -38,185 +31,66 @@ * Subsystem that manages Discord RPC in the game client, such as status or connection. * This subsystem can be enhanced further to improve game presentation in rich presence. * + * It communicates with the thread safely using thread-safe shared buffer. + * * @see EngineSubsystem */ -public class DiscordRPCSubSystem implements EngineSubsystem, IPCListener, Runnable, PropertyChangeListener { - +public final class DiscordRPCSubSystem implements EngineSubsystem, PropertyChangeListener { private static final Logger logger = LoggerFactory.getLogger(DiscordRPCSubSystem.class); - private static final long DISCORD_APP_CLIENT_ID = 515274721080639504L; - private static final String DISCORD_APP_LARGE_IMAGE = "ss_6"; - private static final int RECONNECT_TRIES = 5; private static DiscordRPCSubSystem instance; - private IPCClient ipcClient; - private boolean ready; - private boolean autoReconnect; - private Thread reconnectThread; - private RichPresence lastRichPresence; - private boolean reconnecting; - private int reconnectTries = 1; - private boolean connectedBefore; private Config config; - private String lastState; - private boolean dontTryAgain; - private boolean enabled; + private DiscordRPCThread thread; public DiscordRPCSubSystem() throws IllegalStateException { if (instance != null) { throw new IllegalStateException("More then one instance in the DiscordRPC"); } - lastRichPresence = null; - ipcClient = new IPCClient(DISCORD_APP_CLIENT_ID); - ipcClient.setListener(this); - autoReconnect = true; - reconnectThread = new Thread(this); - reconnectThread.setName("DISCORD-RPC-RECONNECT"); - reconnectThread.start(); - instance = this; - enabled = false; - dontTryAgain = true; - } - - public void sendRichPresence(RichPresence richPresence) { - this.lastRichPresence = richPresence; - if (!ready || lastRichPresence == null || !enabled) { - return; - } - ipcClient.sendRichPresence(lastRichPresence); - } - @Override - public void onReady(IPCClient client) { - if (reconnecting) { - logger.info("Discord RPC >> Reconnected!"); - reconnectTries = 1; - } else { - logger.info("Discord RPC >> Connected!"); - connectedBefore = true; - } - this.ipcClient = client; - if (!ready) { - ready = true; - } - if (lastRichPresence == null) { - RichPresence.Builder builder = new RichPresence.Builder(); - builder.setLargeImage(DISCORD_APP_LARGE_IMAGE); - lastRichPresence = builder.build(); - } - client.sendRichPresence(lastRichPresence); - } - - @Override - public void onDisconnect(IPCClient client, Throwable t) { - if (ready) { - ready = false; - } - logger.info("Discord RPC >> Disconnected!"); + instance = this; } @Override - public void run() { - while (autoReconnect) { - try { - // Ignore if the Discord RPC is not enabled - if (!enabled) { - if (ready) { - getInstance().ipcClient.close(); - } - Thread.sleep(1000); - continue; - } - - // Don't retry to do any connect to the RPC till something happen to do it - if (dontTryAgain) { - Thread.sleep(1000); - continue; - } + public void initialise(GameEngine engine, Context rootContext) { + logger.info("Initializing..."); - // Connect if the connect on init didn't connect successfully - if (!connectedBefore && !ready) { - try { - ipcClient.connect(); - } catch (Exception ex) { - } // Ignore the not able to connect to continue our process - Thread.sleep(15 * 1000); - if (!ready) { - reconnectTries += 1; - if (reconnectTries >= RECONNECT_TRIES) { - dontTryAgain = true; - } - } - continue; - } + thread = new DiscordRPCThread(); + thread.getBuffer().setState("In Main Menu"); - // Ping to make sure that the RPC is alive - if (ready) { - Thread.sleep(5000); - ipcClient.sendRichPresence(this.lastRichPresence); - } else { - reconnecting = true; - int timeout = (reconnectTries * 2) * 1000; - logger.info("Discord RPC >> Reconnecting... (Timeout: " + timeout + "ms)"); - try { - ipcClient.connect(); - } catch (Exception ex) { - if (reconnectTries <= RECONNECT_TRIES) { - reconnectTries += 1; - } - if (reconnectTries >= RECONNECT_TRIES) { - dontTryAgain = true; - } - Thread.sleep(timeout); - } - } - } catch (InterruptedException ex) { // Ignore the interrupted exceptions - } catch (Exception ex) { - logger.trace(ex.getMessage(), ex.getCause()); - } - } - } + config = rootContext.get(Config.class); - @Override - public void initialise(GameEngine engine, Context rootContext) { - disableLogger(IPCClient.class); - disableLogger(WindowsPipe.class); - disableLogger(Pipe.class); - Config c = rootContext.get(Config.class); - enabled = c.getPlayer().isDiscordPresence(); - if (!enabled) { - return; + if (config.getPlayer().isDiscordPresence()) { + thread.enable(); + } else { + logger.info("Discord RPC is disabled! No connection is being made during initialization."); + thread.disable(); } - try { - logger.info("Discord RPC >> Connecting..."); - ipcClient.connect(); - dontTryAgain = false; - } catch (Exception ex) { } // Ignore due to reconnect thread + thread.start(); } @Override - public void postInitialise(Context context) { + public synchronized void postInitialise(Context context) { config = context.get(Config.class); config.getPlayer().subscribe(this); - setState("In Main Menu", false, false); + + if (config.getPlayer().isDiscordPresence()) { + thread.enable(); + } else { + thread.disable(); + } } @Override - public void preShutdown() { - autoReconnect = false; - reconnectThread.interrupt(); - if (ready) { - ipcClient.close(); + public void propertyChange(PropertyChangeEvent evt) { + if (evt.getPropertyName().equals(PlayerConfig.DISCORD_PRESENCE)) { + thread.setEnabled((boolean) evt.getNewValue()); } } - /** - * To disable the logger from some classes that throw errors and some other spam stuff into our console. - * - */ - private void disableLogger(Class clazz) { - LoggerContext loggerContext = (LoggerContext) LoggerFactory.getILoggerFactory(); - Logger l = loggerContext.getLogger(clazz); - ((ch.qos.logback.classic.Logger) l).setLevel(Level.OFF); + @Override + public synchronized void preShutdown() { + thread.disable(); + thread.stop(); } @Override @@ -224,99 +98,35 @@ public String getName() { return "DiscordRPC"; } - public static DiscordRPCSubSystem getInstance() { - return instance; + /** + * Re-discovers the discord ipc in case the player started the discord client after running the game. + * And, the re-connecting process failed to connect. + * + * This should be called once by {@link DiscordRPCSystem} + */ + public static void discover() { + getInstance().thread.discover(); } + /** + * Sets the current game/party status for the player (e.g. Playing Solo, Idle, etc...) + * + * @param state The current game/party status + */ public static void setState(String state) { - setState(state, true); + getInstance().thread.getBuffer().setState(state); } - public static void setState(String state, boolean timestamp) { - setState(state, timestamp, true); - } - - public static void setState(String state, boolean timestamp, boolean showDetails) { - if (instance == null) { - return; - } - RichPresence.Builder builder = new RichPresence.Builder(); - if (state != null) { - builder.setState(state); - if (getInstance().lastState == null || !getInstance().lastState.equals(state)) { - getInstance().lastState = state; - } - } - if (showDetails && getInstance().config != null) { - String playerName = getInstance().config.getPlayer().getName(); - builder.setDetails("Name: " + playerName); - } - if (timestamp) { - builder.setStartTimestamp(OffsetDateTime.now()); - } - - builder.setLargeImage(DISCORD_APP_LARGE_IMAGE); - getInstance().sendRichPresence(builder.build()); - } - - public static void updateState() { - if (getInstance() == null) { - return; - } - setState(getInstance().lastState); - } - - public static void tryToDiscover() { - if (getInstance() == null) { - return; - } - if (getInstance().dontTryAgain && getInstance().enabled) { - getInstance().dontTryAgain = false; - getInstance().reconnectTries = 0; - } - } - - public static void enable() { - setEnabled(true); - } - - public static void disable() { - setEnabled(false); - } - - public static void setEnabled(boolean enable) { - if (getInstance() == null) { - return; - } - getInstance().enabled = enable; - if (!enable) { - getInstance().reconnectTries = 0; - } else { - tryToDiscover(); - } - } - - public static boolean isEnabled() { - if (getInstance() == null) { - return false; - } - return getInstance().enabled; + /** + * Sets an elapsed time since the player's state + * + * @param timestamp The elapsed time since player's action. `null` to disable it. + */ + public static void setStartTimestamp(OffsetDateTime timestamp) { + getInstance().thread.getBuffer().setStartTimestamp(timestamp); } - @Override - public void propertyChange(PropertyChangeEvent evt) { - if (evt.getPropertyName().equals(PlayerConfig.DISCORD_PRESENCE)) { - boolean discordPresence = (boolean) evt.getNewValue(); - if (isEnabled() != discordPresence) { - if (discordPresence) { - enable(); - } else { - disable(); - } - } - } - if (evt.getPropertyName().equals(PlayerConfig.PLAYER_NAME)) { - updateState(); - } + private static DiscordRPCSubSystem getInstance() { + return instance; } } diff --git a/subsystems/DiscordRPC/src/main/java/org/terasology/engine/subsystem/discordrpc/DiscordRPCSystem.java b/subsystems/DiscordRPC/src/main/java/org/terasology/engine/subsystem/discordrpc/DiscordRPCSystem.java index 83c7d55fe89..b30d2b465c4 100644 --- a/subsystems/DiscordRPC/src/main/java/org/terasology/engine/subsystem/discordrpc/DiscordRPCSystem.java +++ b/subsystems/DiscordRPC/src/main/java/org/terasology/engine/subsystem/discordrpc/DiscordRPCSystem.java @@ -14,6 +14,8 @@ import org.terasology.network.NetworkSystem; import org.terasology.registry.In; +import java.time.OffsetDateTime; + /** * It's a system that runs when a single player or multi player game has been started to process some stuff throw the * {@link DiscordRPCSubSystem}. @@ -21,7 +23,7 @@ * @see DiscordRPCSubSystem */ @RegisterSystem(RegisterMode.CLIENT) -public class DiscordRPCSystem extends BaseComponentSystem { +public final class DiscordRPCSystem extends BaseComponentSystem { @In private Game game; @@ -32,61 +34,56 @@ public class DiscordRPCSystem extends BaseComponentSystem { @In private NetworkSystem networkSystem; - public String getGame() { - NetworkMode networkMode = networkSystem.getMode(); - String mode = "Playing Online"; - if (networkMode == NetworkMode.DEDICATED_SERVER) { - mode = "Hosting | " + game.getName(); - } else if (networkMode == NetworkMode.NONE) { - mode = "Solo | " + game.getName(); - } - return mode; - } - - @ReceiveEvent - public void onAfk(AfkEvent event, EntityRef entityRef) { - if (requireConnection() && player.getClientEntity().equals(entityRef)) { - return; - } - if (event.isAfk()) { - disableDiscord(); - } else { - enableDiscord(); - } - } - - private boolean requireConnection() { - NetworkMode networkMode = networkSystem.getMode(); - return networkMode != NetworkMode.CLIENT && networkMode != NetworkMode.DEDICATED_SERVER; - } - - private void enableDiscord() { - DiscordRPCSubSystem.tryToDiscover(); - DiscordRPCSubSystem.setState("Idle", true); - } - - private void disableDiscord() { - DiscordRPCSubSystem.tryToDiscover(); - DiscordRPCSubSystem.setState(getGame(), true); - } - @Override public void initialise() { - DiscordRPCSubSystem.tryToDiscover(); + DiscordRPCSubSystem.discover(); } @Override public void preBegin() { - DiscordRPCSubSystem.setState(getGame(), false); + DiscordRPCSubSystem.setState(getGame()); + DiscordRPCSubSystem.setStartTimestamp(null); } @Override public void postBegin() { - DiscordRPCSubSystem.setState(getGame(), true); + DiscordRPCSubSystem.setStartTimestamp(OffsetDateTime.now()); } @Override public void shutdown() { - DiscordRPCSubSystem.setState("In Main Menu", false, false); + DiscordRPCSubSystem.setState("In Main Menu"); + DiscordRPCSubSystem.setStartTimestamp(null); + } + + @ReceiveEvent + public void onAfk(AfkEvent event, EntityRef entityRef) { + if (isServer() && player.getClientEntity().equals(entityRef)) { + return; + } + + if (event.isAfk()) { + DiscordRPCSubSystem.setState("Idle"); + DiscordRPCSubSystem.setStartTimestamp(null); + } else { + DiscordRPCSubSystem.setState(getGame()); + DiscordRPCSubSystem.setStartTimestamp(OffsetDateTime.now()); + } + } + + public String getGame() { + NetworkMode networkMode = networkSystem.getMode(); + String mode = "Playing Online"; + if (networkMode == NetworkMode.DEDICATED_SERVER) { + mode = "Hosting | " + game.getName(); + } else if (networkMode == NetworkMode.NONE) { + mode = "Solo | " + game.getName(); + } + return mode; + } + + private boolean isServer() { + NetworkMode networkMode = networkSystem.getMode(); + return networkMode != NetworkMode.CLIENT && networkMode != NetworkMode.DEDICATED_SERVER; } } diff --git a/subsystems/DiscordRPC/src/main/java/org/terasology/engine/subsystem/discordrpc/DiscordRPCThread.java b/subsystems/DiscordRPC/src/main/java/org/terasology/engine/subsystem/discordrpc/DiscordRPCThread.java new file mode 100644 index 00000000000..0d579e8906e --- /dev/null +++ b/subsystems/DiscordRPC/src/main/java/org/terasology/engine/subsystem/discordrpc/DiscordRPCThread.java @@ -0,0 +1,250 @@ +// Copyright 2020 The Terasology Foundation +// SPDX-License-Identifier: Apache-2.0 + +package org.terasology.engine.subsystem.discordrpc; + +import com.jagrosh.discordipc.IPCClient; +import com.jagrosh.discordipc.IPCListener; +import com.jagrosh.discordipc.entities.RichPresence; +import com.jagrosh.discordipc.exceptions.NoDiscordClientException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public final class DiscordRPCThread implements IPCListener, Runnable { + private static final Logger logger = LoggerFactory.getLogger(DiscordRPCThread.class); + private static final long DISCORD_APP_CLIENT_ID = 515274721080639504L; + private static final String DISCORD_APP_DEFAULT_IMAGE = "ss_6"; + private static final int MAX_RECONNECT_TRIES = 5; + + private final Thread thread; + private final IPCClient ipcClient; + private final DiscordRPCBuffer buffer; + private RichPresence lastRichPresence; + + private int tries; + + private boolean enabled; + private boolean waiting; + private boolean connectedBefore; + private boolean connected; + private boolean autoReconnect; + + public DiscordRPCThread() { + thread = new Thread(this); + thread.setName("DISCORD-RPC-THREAD"); + + ipcClient = new IPCClient(DISCORD_APP_CLIENT_ID); + ipcClient.setListener(this); + + buffer = new DiscordRPCBuffer(); + + lastRichPresence = null; + + tries = 0; + + enabled = false; + waiting = false; + connectedBefore = false; + connected = false; + autoReconnect = false; + } + + public void start() { + thread.start(); + } + + public synchronized void stop() { + thread.interrupt(); + } + + public synchronized void discover() { + if (enabled && connected) { + return; + } + + reset(); + + connectedBefore = true; + } + + public synchronized void enable() { + if (enabled) { + return; + } + + enabled = true; + + reset(); + + if (waiting && thread.isAlive()) { + synchronized (thread) { + thread.notify(); + } + } + } + + public synchronized void disable() { + if (!enabled) { + return; + } + + enabled = false; + + reset(); + autoReconnect = false; + + if (waiting && thread.isAlive()) { + synchronized (thread) { + thread.notify(); + } + } + } + + @Override + public void onReady(IPCClient ignored) { + if (connectedBefore) { + logger.info("Re-connected to Discord RPC!"); + } else { + logger.info("Connected to Discord RPC!"); + } + + connectedBefore = true; + connected = true; + } + + @Override + public void onDisconnect(IPCClient client, Throwable t) { + connected = false; + logger.info("Discord RPC lost connection: Disconnected!"); + } + + @Override + public void run() { + while (true) { + logger.info("Waiting for auto-connecting..."); + /* If auto-connect is disabled the thread won't get notified*/ + while (!autoReconnect) { + try { + synchronized (thread) { + waiting = true; + thread.wait(); + waiting = false; + } + } catch (InterruptedException ignored) { + return; // End when the thread is being interrupted + } + } + + logger.info("Waiting for enabling..."); + /* Check if the subsystem is enabled */ + while (!enabled) { + try { + synchronized (thread) { + waiting = true; + thread.wait(); + waiting = false; + } + } catch (InterruptedException ignored) { + return; // End when the thread is being interrupted + } + } + + logger.info("Waiting for connection..."); + /* Auto-Connect to the IPC with reconnect process */ + while (!connected) { + synchronized (ipcClient) { + try { + if (!connectedBefore) { + logger.info("Connecting to Discord RPC..."); + } else { + logger.info("Re-connecting to Discord RPC..."); + } + + ipcClient.connect(); + + tries = 0; + autoReconnect = true; + } catch (NoDiscordClientException ignored) { + // TODO implement reconnect process + if (tries >= MAX_RECONNECT_TRIES) { + autoReconnect = false; + tries = 0; + break; + } else { + tries++; + try { + Thread.sleep(2000L * tries); + } catch (InterruptedException ignored2) { + ipcClient.close(); + return; // End when the thread is being interrupted + } + + // Retry to connect again + } + } + } + } + + /* Go to the beginning to trigger auto reconnect loop */ + if (!autoReconnect) { + continue; + } + + logger.info("Updating the rich presence and keep the connection alive..."); + /* Update the rich presence and keeping the connection alive */ + while (connected) { + synchronized (this) { + /* Allocate a new rich presence when the buffer has changed */ + if (enabled && buffer.hasChanged()) { + lastRichPresence = build(); + buffer.resetState(); + } + + /* Ping the ipc connection with an rich presnece to keep the connection alive */ + if (enabled) { + ipcClient.sendRichPresence(lastRichPresence); + } else { + ipcClient.sendRichPresence(null); + } + } + + try { + Thread.sleep(5000); + } catch (InterruptedException ignored) { + synchronized (ipcClient) { + ipcClient.close(); + } + return; + } + } + } + } + + public synchronized void setEnabled(boolean enabled) { + this.enabled = enabled; + } + + public synchronized boolean isEnabled() { + return enabled; + } + + public synchronized DiscordRPCBuffer getBuffer() { + return buffer; + } + + private RichPresence build() { + return new RichPresence.Builder() + .setLargeImage(DISCORD_APP_DEFAULT_IMAGE) + .setState(buffer.getState()) + .setStartTimestamp(buffer.getStartTimestamp()) + .build(); + } + + private void reset() { + tries = 0; + + autoReconnect = true; + connectedBefore = false; + connected = false; + } +}