Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions docs/framework_events.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,29 @@
Custom Events
=============

Bank
^^^^

.. py:method:: red_bank_set_global(global_state: bool)
Dispatched when the Bank gets toggled between Global and Local. :code:`global_state` will be True if the Bank is being set to Global or False if the bank is being set to Local

.. py:method:: red_bank_set_balance(payload: redbot.core.bank.BankSetBalanceInformation)
Dispatched when a user has their balance changed.
Comment on lines +14 to +16
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documented events that dispatch custom objects should link to those objects in some way. It might make sense to use a similar format as the autogenerated API descriptions for methods, which have a "parameters" section describing each of the parameters in detail.


.. py:method:: red_bank_transfer_credits(payload: redbot.core.bank.BankTransferInformation)
Dispatched when credits gets transfered from one user to another.

.. py:method:: red_bank_wipe(guild_id: int)
Dispatched when the Bank gets wiped. :code:`guild_id` will be the ID of the Guild that was wiped, 0 if all Guilds were wiped (Bank is Local), or -1 if all Users were wiped (Bank is Global)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having both 0 and -1 as special values seems a bit smelly. Is there a meaningful difference for the CC to account for that makes differentiating between wiping guilds or users something that needs to happen? If so, I think it would be neater to just have a second argument, whether or not the bank was global at the time of wiping.

I would say the CC can just check if the bank is global or not to get the same value, but that only applies for [p]bankset reset. [p]bankset toggleglobal also implicitly results in this being called, since it clears the bank during the process of swapping scope. Trying to test for it at that time would likely result in an incorrect value due to race conditions.


.. py:method:: red_bank_prune_accounts(payload: redbot.core.bank.BankPruneInformation)
Dispatched when users get pruned from the Bank

RPC Server
^^^^^^^^^^

Expand Down
64 changes: 60 additions & 4 deletions redbot/core/bank.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
import asyncio
import logging
from datetime import datetime, timezone
from typing import Union, List, Optional, TYPE_CHECKING, Literal
from typing import Union, List, Optional, TYPE_CHECKING, Literal, Dict, NamedTuple
from functools import wraps

import discord
from discord import user

from redbot.core.utils import AsyncIter
from redbot.core.utils.chat_formatting import humanize_number
Expand Down Expand Up @@ -81,9 +82,13 @@
_cache_is_global = None
_cache = {"bank_name": None, "currency": None, "default_balance": None, "max_balance": None}

_bot_ref: Optional[Red] = None

async def _init():

async def _init(bot: Red):
global _config
global _bot_ref
_bot_ref = bot
_config = Config.get_conf(None, 384734293238749, cog_name="Bank", force_registration=True)
_config.register_global(**_DEFAULT_GLOBAL)
_config.register_guild(**_DEFAULT_GUILD)
Expand Down Expand Up @@ -336,6 +341,7 @@ async def set_balance(member: Union[discord.Member, discord.User], amount: int)
group = _config.user(member)
else:
group = _config.member(member)
old_balance = await group.balance()
await group.balance.set(amount)

if await group.created_at() == 0:
Expand All @@ -345,6 +351,11 @@ async def set_balance(member: Union[discord.Member, discord.User], amount: int)
if await group.name() == "":
await group.name.set(member.display_name)

payload = BankSetBalanceInformation(
member.id, (0 if _cache_is_global else guild.id), old_balance, amount
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's more typical for guild like attributes to be Optional and use None as the value to represent non-guild actions (IE, message.guild). If it is consistently possible to provide the full user/guild object rather than an ID, I also think that would be preferred.

)

_bot_ref.dispatch("red_bank_set_balance", payload)
return amount


Expand Down Expand Up @@ -483,8 +494,15 @@ async def transfer_credits(
user=to.display_name, max_balance=max_bal, currency_name=currency
)

await withdraw_credits(from_, amount)
return await deposit_credits(to, amount)
sender_new = await withdraw_credits(from_, amount)
recipient_new = await deposit_credits(to, amount)

payload = BankTransferInformation(
amount, from_.id, to.id, (0 if _cache_is_global else guild.id), sender_new, recipient_new
)
_bot_ref.dispatch("red_bank_transfer_credits", payload)

return recipient_new


async def wipe_bank(guild: Optional[discord.Guild] = None) -> None:
Expand All @@ -499,8 +517,10 @@ async def wipe_bank(guild: Optional[discord.Guild] = None) -> None:
"""
if await is_global():
await _config.clear_all_users()
_bot_ref.dispatch("red_bank_wipe", -1)
else:
await _config.clear_all_members(guild)
_bot_ref.dispatch("red_bank_wipe", getattr(guild, "id", 0))


async def bank_prune(bot: Red, guild: discord.Guild = None, user_id: int = None) -> None:
Expand All @@ -524,6 +544,14 @@ async def bank_prune(bot: Red, guild: discord.Guild = None, user_id: int = None)

"""

print(f"{guild} | {user_id}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leftover print?

if guild is None and user_id is None:
scope = 1 # prune global
if guild is not None and user_id is None:
scope = 2 # prune server
else:
scope = 3 # prune user
Comment on lines +548 to +553
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to hit submit on the comment I was writing here....

Doing it like this loses the ability to know what guild was being pruned. Unless there is some reason to differentiate between specifically pruning a member and a single member happening to be the only invalid member from a bulk prune, the signature of this event object would likely be better as:
guild: Optional[discord.Guild], pruned_user_ids: List[int]


global_bank = await is_global()

if global_bank:
Expand Down Expand Up @@ -563,6 +591,10 @@ async def bank_prune(bot: Red, guild: discord.Guild = None, user_id: int = None)
if user_id in bank_data:
del bank_data[user_id]

payload = BankPruneInformation(scope, tmp)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tmp is only defined when user_id is None. This errors if a particular user is pruned.
(I do think the conditionally defined variables in this function are smelly, making this sort of error very easy to make)


_bot_ref.dispatch("red_bank_prune_accounts", payload)


async def get_leaderboard(positions: int = None, guild: discord.Guild = None) -> List[tuple]:
"""
Expand Down Expand Up @@ -724,11 +756,14 @@ async def set_global(global_: bool) -> bool:

if await is_global():
await _config.clear_all_users()
_bot_ref.dispatch("red_bank_wipe", -1)
else:
await _config.clear_all_members()
_bot_ref.dispatch("red_bank_wipe", 0)

await _config.is_global.set(global_)
_cache_is_global = global_
_bot_ref.dispatch("red_bank_set_global", global_)
return global_


Expand Down Expand Up @@ -1085,3 +1120,24 @@ async def wrapped(*args, **kwargs):
return coro_or_command

return deco


class BankTransferInformation(NamedTuple):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These objects should be added to __all__, since they are part of the public API. They also need to be documented, which may or may not happen automatically after doing so.

transfer_amount: int
sender_id: int
recipient_id: int
guild_id: int
sender_new_balance: int
recipient_new_balance: int


class BankSetBalanceInformation(NamedTuple):
recipient_id: int
guild_id: int
recipient_old_balance: int
recipient_new_balance: int


class BankPruneInformation(NamedTuple):
scope: int
pruned_users: Union[List[int], Dict[str, List[int]]]
2 changes: 1 addition & 1 deletion redbot/core/bot.py
Original file line number Diff line number Diff line change
Expand Up @@ -1154,7 +1154,7 @@ async def _pre_connect(self) -> None:
await self.add_cog(Dev())

await modlog._init(self)
await bank._init()
await bank._init(self)

packages = OrderedDict()

Expand Down
4 changes: 2 additions & 2 deletions redbot/pytest/economy.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@


@pytest.fixture()
async def bank(config, monkeypatch):
async def bank(config, monkeypatch, red):
from redbot.core import Config

with monkeypatch.context() as m:
m.setattr(Config, "get_conf", lambda *args, **kwargs: config)
# noinspection PyProtectedMember
await bank_module._init()
await bank_module._init(red)
return bank_module