Shopkeepers

Shopkeepers

2M Downloads

Offline support for trade / shop notifications for shop owners

blablubbabc opened this issue ยท 3 comments

commented

From #598

One option would be to store the notifications while the player is offline.

However, trade notifications would probably not need to be stored once we have support for keeping track of the complete trade history.
Also, instead of storing out-of-stock / chest-full notifications, there could also be some kind of procedure that checks the status of the player's shops whenever the player joins the server.

commented

Hey, players on my server want this feature. I am fine implementing this myself and creating a pull request for it, but I would like to clarify one thing - trade storage. Right now, trades are optionally logged to a csv file. I was thinking that, in order to create a feature like this, it'd make more sense to store logs in a local database (H2 or SQLite, leaning towards the latter as Spigot comes with the SQLite driver). That way one could query trades more dynamically, and one could also create commands to query - both for players and administrators - and prune this database. Does this sound like a wanted change, or would you prefer to continue working with CSV?
For simplicity, the first PR would only include changing storage type (if the above sounds good to you), then separate PRs would be made to add a use-logs-to-notify-players-of-offline-trades configuration options and for management commands.

commented

Hey, players on my server want this feature. I am fine implementing this myself and creating a pull request for it, but I would like to clarify one thing - trade storage. Right now, trades are optionally logged to a csv file. I was thinking that, in order to create a feature like this, it'd make more sense to store logs in a local database (H2 or SQLite, leaning towards the latter as Spigot comes with the SQLite driver). That way one could query trades more dynamically, and one could also create commands to query - both for players and administrators - and prune this database. Does this sound like a wanted change, or would you prefer to continue working with CSV? For simplicity, the first PR would only include changing storage type (if the above sounds good to you), then separate PRs would be made to add a use-logs-to-notify-players-of-offline-trades configuration options and for management commands.

I would lean towards a sqlite based solution as well.

I actually started implementing something like this in the past in order to create a trade history command (f1ec428), but I never completed this, and I think the scope was maybe a bit too broad there, making the solution more complex than it probably needs to be for a first iteration (the idea was to also support mysql storage, to store the data in a normalized form to avoid duplication, and maybe even allow storing the data of different servers in the same database for cross-server trade histories, and to also store player meta data).

But let's keep the initial implementation simple:

  • Let's focus on local trading history data only, and ignore any other potential use cases besides the trade notifications and trade history command.
  • Let's maybe use a simple non-normalized form to log the data, i.e. a 1-to-1 equivalent to the current csv format. The implementation can be oriented on the current csv trade logger.
  • Since unlike with the csv trade logs, plugin users usually don't access the sqlite data directly, let's maybe store all timestamps in UTC, and later convert them to the current local timezone/format when used for display purposes.
  • Let's ignore versioning and data schema migrations for now. This can still be looked into once the first schema update is actually required.
  • Maybe let's replace the current log-trades-to-csv with a generic trade-log-storage setting with valid values 'csv' and 'sqlite', to allow for some extensibility there in the future. We could even sqitch to sqlite as the default storage format then. And we would need a separate new enable-trade-log: false then (I would keep this disabled by default until we have also added some automatic purging feature that is enabled by default).
    • This can be implemented via a config migration. See package com.nisovin.shopkeepers.config.migration for examples.
    • I don't think we need to support logging to multiple storage formats at the same time. I know that there are some servers that have built their own tools around the csv output, such as for showing the trade history on their website. But it should probably not be too complicated for them to migrate their tools to read the data from the sqlite database instead.

Like you mentioned, features like the trade history command, offline trade notifications, automatic purging of history items older than x days, etc. can be looked into subsequently. But I would probably opt for a simpler setting than use-logs-to-notify-players-of-offline-trades then. Maybe simply notify-shop-owners-about-offline-trades with a comment in the default config mentioning that this requires trade-log-storage: 'sqlite', as well as a warning during config loading if the offline notifications are enabled, but the log storage is disabled or not using a compatible storage type.

commented

Thanks for the quick response, all of that sounds good to me! I'll try to implement the additional TradeLogger throughout this week then :)