duelyst icon indicating copy to clipboard operation
duelyst copied to clipboard

[P3] Deduplicate functions in Game and SP code

Open willroberts opened this issue 3 years ago • 4 comments

Nearly every function is server/game.coffee is duplicated in server/single_player.coffee:

dnsHealthCheck = () ->
server = http.createServer (req, res) ->
server.listen config.get('game_port'), () ->
savePlayerCount = (playerCount) ->
saveGameCount = (gameCount) ->
getConnectedSpectatorsDataForGamePlayer = (gameId,playerId)->
onGamePlayerJoin = (requestData) ->
onGameSpectatorJoin = (requestData) ->
onGameLeave = (requestData) ->
onGameEvent = (eventData) ->
onGameDisconnect = () ->
playerLeaveGameIfNeeded = (socket, silent=false) ->
spectatorLeaveGameIfNeeded = (socket) ->
destroyGameSessionIfNoConnectionsLeft = (gameId,persist=false)->
tearDownSpectateSystemsIfNoSpectatorsLeft = (gameId)->
clearDisconnectedPlayerTimeout = (gameId) ->
startDisconnectedPlayerTimeout = (gameId,playerId) ->
onDisconnectedPlayerTimeout = (gameId,playerId) ->
restartTurnTimer = (gameId) ->
stopTurnTimer = (gameId) ->
onGameTimeTick = (gameId) ->
restartSpectatorDelayedGameInterval = (gameId) ->
stopSpectatorDelayedGameInterval = (gameId) ->
onSpectatorDelayedGameTick = (gameId) ->
flushSpectatorNetworkEventBuffer = (gameId) ->
_logSpectatorTickInfo = _.debounce((gameId)->
emitGameEvent = (fromSocket,gameId,eventData)->
initGameSession = (gameId,onComplete) ->
initSpectatorGameSession = (gameId)->
onBeforeRollbackToSnapshot = (event) ->
onStep = (event) ->
onInvalidAction = (event) ->
subscribeToGameSessionEvents = (gameId)->
unsubscribeFromGameSessionEvents = (gameId)->
afterGameOver = (gameId, gameSession, mouseAndUIEvents) ->
shutdown = () ->

We should deduplicate as much of this as possible to make maintenance easier.

willroberts avatar Sep 23 '22 13:09 willroberts

A lot of the code here is indeed duplicated but there may be subtle differences. I believe the initial implementation of the single player server was an experiment and we had just copy-pasted the code but would make sense to convert into a shared library.

marwanhilmi avatar Sep 23 '22 13:09 marwanhilmi

Definitely! I'll diff each of these one at a time and see what we can do.

willroberts avatar Sep 23 '22 13:09 willroberts

I wrote a Python script to diff these automatically:

from typing import Dict, List

# Identifies CoffeeScript functions through signature and indentation.
def find_functions(filename: str) -> Dict[str, List[str]]:
    start_index = -1
    functions: Dict[str, List[str]] = dict()

    with open(filename, 'r') as f:
        lines = f.readlines()

    for i in range(0, len(lines)):
        line = lines[i]

        # Ignore comments and lines starting with whitespace.
        if line[0] in ('#', '\t', ' '): continue

        # Save the starting line for function declarations.
        if '->' in line and ' = (' in line:
            start_index = i
            continue

        # Don't try to find the end of a function before one begins.
        if start_index == -1: continue

        # Something new has started at the top level; save and reset.
        functions[lines[start_index].rstrip('\n')] = lines[start_index:i]
        start_index = -1

    return functions

# Returns True if the provided lists have identical contents.
def diff_function(a: List[str], b: List[str]) -> bool:
    if len(a) != len(b):
        return False
    for i in range(len(a)):
        if a[i] != b[i]:
            return False
    return True

if __name__ == '__main__':
    game = find_functions('game.coffee')
    sp = find_functions('single_player.coffee')
    for f in game.keys() & sp.keys():
        if diff_function(game[f], sp[f]):
            print('Function with declaration {} is duplicated!'.format(f))

The results are:

Function with declaration afterGameOver = (gameId, gameSession, mouseAndUIEvents) -> is duplicated!
Function with declaration destroyGameSessionIfNoConnectionsLeft = (gameId,persist=false)-> is duplicated!
Function with declaration dnsHealthCheck = () -> is duplicated!
Function with declaration flushSpectatorNetworkEventBuffer = (gameId) -> is duplicated!
Function with declaration getConnectedSpectatorsDataForGamePlayer = (gameId,playerId)-> is duplicated!
Function with declaration initGameSession = (gameId,onComplete) -> is duplicated!
Function with declaration initSpectatorGameSession = (gameId)-> is duplicated!
Function with declaration onGameEvent = (eventData) -> is duplicated!
Function with declaration onGamePlayerJoin = (requestData) -> is duplicated!
Function with declaration onGameSpectatorJoin = (requestData) -> is duplicated!
Function with declaration onGameTimeTick = (gameId) -> is duplicated!
Function with declaration onInvalidAction = (event) -> is duplicated!
Function with declaration onSpectatorDelayedGameTick = (gameId) -> is duplicated!
Function with declaration onStep = (event) -> is duplicated!
Function with declaration playerLeaveGameIfNeeded = (socket, silent=false) -> is duplicated!
Function with declaration restartSpectatorDelayedGameInterval = (gameId) -> is duplicated!
Function with declaration restartTurnTimer = (gameId) -> is duplicated!
Function with declaration saveGameCount = (gameCount) -> is duplicated!
Function with declaration savePlayerCount = (playerCount) -> is duplicated!
Function with declaration shutdown = () -> is duplicated!
Function with declaration spectatorLeaveGameIfNeeded = (socket) -> is duplicated!
Function with declaration stopSpectatorDelayedGameInterval = (gameId) -> is duplicated!
Function with declaration stopTurnTimer = (gameId) -> is duplicated!
Function with declaration tearDownSpectateSystemsIfNoSpectatorsLeft = (gameId)-> is duplicated!

willroberts avatar Sep 25 '22 01:09 willroberts

Another challenge is the reliance on global vars, specifically these two:

  • io object: https://github.com/open-duelyst/duelyst/blob/fa4184f/server/single_player.coffee#L73
  • games object: https://github.com/open-duelyst/duelyst/blob/fa4184f/server/single_player.coffee#L95

Functions accessing these (nearly all of the above) would need to be refactored or remain duplicated.

I made some progress on this work here: https://github.com/willroberts/duelyst/compare/main...willroberts:duelyst:server-dedupe

Given the io/games access challenges I'll hold off on this for now.

willroberts avatar Sep 25 '22 16:09 willroberts