Z1ni / XGP-save-extractor

Python script to extract savefiles out of Xbox Game Pass for PC games
MIT License
563 stars 36 forks source link

Suggestion: Code Performance & Scalability Improvements #99

Open SquirrelsMcGee opened 8 months ago

SquirrelsMcGee commented 8 months ago

Hi I just used this tool and found it really useful and was wondering how the code actually worked

It was all very interesting but I've noticed an issue that is starting to creep up as more games get supported

In main.py in the get_save_paths function, it looks like you're pulling from a list of known handlers to get the name and what required arguments each handler needs, but then you are performing a series of if-elif statements to perform the extraction for each game, this isn't really scalable, as more games gets added, the script will just keep getting bigger and more complex, ideally you would separate out the handler implementation from the rest of the code

A small code quality improvement you could make here is to move the content of each block into it's own seperate function / class, then register each of those in a dictionary using the handler_key as the key

psuedo-code example:

# Base handler defintion
class BaseHandler:
  def __init__(name):
    self.name = name
  def handle(args, containers, save_meta):
    # this is where you could expect to do the content of the if block discussed above

# Example of subclass
class PalworldHandler:
  def __init__():
    super("palworld")

  def handle(args, containers, save_meta):
    for container in containers:
      fname = container["name"]
      # Each "-" in the name is a directory separator
      fname = fname.replace("-", "/")
      fname += ".sav"
      fpath = container["files"][0]["path"]
      save_meta.append((fname, fpath))

# You would then register each of these subclasses in a dictionary like so
handlers_implementation = {
  "palworld": PalworldHandler(),
  "someotherone": SomeOtherHandler()
}

# then lookup in the handlers_implementation dictionary when checking if a handler exists, 
# or provide a handler that does nothing for the empty string case
SquirrelsMcGee commented 8 months ago

Note: I haven't actually built this on my end, as i'm writing this during my lunch break at work 😛

Z1ni commented 8 months ago

Yeah, there's a lot of refactoring possibilities. I just moved the game directory to a JSON file, but of course I didn't touch the hacky handling side much.

This is what happens when you write a hacky script for one game and then try to add more games without refactoring the whole script :sweat_smile: