RanvierMUD / core

Core engine code for Ranvier
https://ranviermud.com
MIT License
45 stars 40 forks source link

Datastorage is semi-hardcoded for accounts #96

Open Zuzuesque opened 5 years ago

Zuzuesque commented 5 years ago

While the documentation suggests it is enough to switch out the EntityLoader to implement your custom data source, this is currently not entirely true for accounts. You can load them from any alternate source, but the only proper method for creating/saving them is calling save() on the Account object (as done by the example bundles). Save() on Account.js in turn calles the save() function on Data.js, which currently looks like this:

 /**
   * Save data file (player/account) data to disk
   * @param {string} type
   * @param {string} id
   * @param {*} data
   * @param {function} callback
   */
  static save(type, id, data, callback) {
    fs.writeFileSync(this.getDataFilePath(type, id), JSON.stringify(data, null, 2), 'utf8');
    if (callback) {
      callback();
    }
  }

... regardless of the user implementation for the data source.

Work Around To implement custom sources for accounts there are currently two ways--switching out Account.js with one imlementing a custom save() function (and consequently AccountManager which depends on it)--or to access the loader stored in AccountManager directly in your code via Accountmanager.loader.custom_loader_save_function(). The first is somewhat overkill if you have no intention of changing the fields/pw encryption Account provides while the later leaves it to the developer to make sure the loader they access from the Accountmanager is the one they hooked up and feels unintuitive.

Proposed Solution:

Optional Consideration: EntityLoader currently has implementations for update/replace which are not used by AccountManager and inaccessible unless using the loader directly. There is also no delete function for cases when you do want to delete an account properly rather than just setting account.delete to true. It may be an interesting feature to add the full set of CRUD functions to the AccountManager so it can serve as a gateway to accessing all loader functions for usecases where people really only want to change how data is saved/loaded, but don't need custom implementations for accounts.

shawncplus commented 5 years ago

Add a create/save function to the EntityLoader (similar to the already existing update and replace functions),

There's no need for 4 methods. CRU can all be accomplished with the current EntityLoader architecture. It might need a remove added though

--

To all the rest that's the plan, it was mentioned in another ticket but the Data class serves no good purpose now with the EntityLoader system.

NuSkooler commented 4 years ago

Any ETA on 3.2 where this will be fixed? I just ran into this when attempting to make case insensitive JSON based data/directory source (e.g. case insensitive account & player names/lookup).

azigler commented 4 years ago

You can fix this by appending the AccountManager to Account when adding it to the manager (I used the this.__manager to be consistent with Item). Then change the Account.save() method like so:

/**
   * @param {function} callback after-save callback
   */
  save(callback) {
    this.__manager.loader.update(this.username, this.serialize())
      .then(() => callback());
  }
NuSkooler commented 3 years ago

@azigler Can you give a little more detail on what you're describing above? I looked at the commit referenced here but that doesn't seem to be enough to make it work.

EDIT: Wait, I think you just mean monkey patch in __manager, e.g.:

args.account.__manager = state.AccountManager;

?