Ardent Andromedas #1
Conversation
…ordpy-integration # Conflicts: # src/bot.py # src/settings.py
…ownload latency / cacheing.
- Refactor SQLite database for storing events and guild configurations - Implement Database class with methods for initializing, inserting, and retrieving data - Update EcoCordClient to use the new Database class - Modify ConfigureModal to save guild configuration to the database - Update channel reconfiguration process to use stored configurations - Add .sqlite3 files to .gitignore
- Replace ConfigureModal with ConfigureView for better UX - Implement periodic online critter updates - Enhance error handling and logging in Database class - Fix issues with event processing and critter management - Improve type hints and docstrings - Sync slash commands for each guild on bot startup
- Integrate WordCloud library for generating word clouds - Create WordCloudObject class for rendering word clouds in the ecosystem - Add word cloud rendering to Ecosystem class - Implement stop_all_ecosystems method in EcoCordClient - Update cleanup process in Ecosystem and EcosystemManager - Combine 'ruff check' and 'ruff format' commands in lint script - Remove separate format script - Minor code cleanup and formatting improvements
- Implement SpeechBubble class for displaying critter messages - Add speech bubble handling in Ecosystem and EcosystemManager - Simulate random messages for interactive mode - Update imports and minor refactoring
- Remove tail drawing functionality - Implement rounded rectangle design for bubbles - Optimize surface creation and drawing process - Simplify position updating
- Add EcosystemState and UserInfo classes - Implement avatar and role color support for critters - Add methods for saving and retrieving ecosystem state - Improve database initialization and user info handling - Update event processing to include user info - Refactor gif generation process for better error handling
There was a problem hiding this comment.
The style of the code and the way it's organized is excellent other than a few points here and there. I had relatively little to say because most of the code is pretty much how I would have written it. Your README is great.
The commit messages are mostly good, but some could use work. This is a good guide: https://cbea.ms/git-commit/
Everything is type-hinted nicely. Doc-strings are a little inconsistent.
I think you could do a better job of splitting the ecosystem graphical stuff, event handling stuff, and GIF generation in ecosystem.py from each other.
I did run into two semi-serious errors: one getting the configure command registered and the other in stopping ecosystems by reconfiguring. See those comments for more detail.
I do have some concerns about the scalability of running a pygame instance for each channel the bot is listening to, but I get that this was written in a week and not necessarily meant to scale.
The project idea is pretty cute and overall you did a great job executing on it.
| self.managed_channels = [ | ||
| channel for channel in visible_channels if channel.id in guild_data.get("ecosystem_managers", {}) | ||
| ] | ||
| self.gif_channel = next( |
There was a problem hiding this comment.
It probably would have been cleaner to pass guild instead of guild.id to ConfigureView, then you can access guild.id and guild.get_channel
|
|
||
| while not self.command_queue.empty(): | ||
| command, args = self.command_queue.get() | ||
| if command == "stop": |
There was a problem hiding this comment.
I think there's an error here. In response to the "stop" command this sets self.running which controls the outer loop, but the inner loop is controlled by the command queue, so while things are still being added to the queue from the main process, neither loop will end.
| def _remove_critter(self, ecosystem: Ecosystem, user_id: int) -> None: | ||
| if user_id in self.user_critters: | ||
| critter = self.user_critters.pop(user_id) | ||
| ecosystem.critters.discard(critter) |
There was a problem hiding this comment.
ecosystem.critters is a list, which doesn't have a discard method
| timestamp=datetime.now(UTC), | ||
| guild=self.fake_guild, | ||
| channel=channel, | ||
| user=user, |
There was a problem hiding this comment.
parameter should be member, I think
|
|
||
| from bot.settings import BOT_TOKEN_ARRAY as TOKENS | ||
|
|
||
| random_messages: list[str] = [ |
There was a problem hiding this comment.
Type checkers should be able to just infer this, so the type hint isn't needed
| channel=SerializableTextChannel(channel.id, channel.name) | ||
| if channel and hasattr(channel, "id") and hasattr(channel, "name") | ||
| else None, | ||
| member=SerializableMember( | ||
| member.id, | ||
| member.name, | ||
| member.display_name, | ||
| [role.id for role in member.roles], | ||
| member.guild.id, | ||
| str(member.avatar.url) if member.avatar else None, | ||
| str(member.color), | ||
| ) | ||
| if member | ||
| else None, |
There was a problem hiding this comment.
to make this easier to read
| channel=SerializableTextChannel(channel.id, channel.name) | |
| if channel and hasattr(channel, "id") and hasattr(channel, "name") | |
| else None, | |
| member=SerializableMember( | |
| member.id, | |
| member.name, | |
| member.display_name, | |
| [role.id for role in member.roles], | |
| member.guild.id, | |
| str(member.avatar.url) if member.avatar else None, | |
| str(member.color), | |
| ) | |
| if member | |
| else None, | |
| channel=( | |
| SerializableTextChannel(channel.id, channel.name) | |
| if channel and hasattr(channel, "id") and hasattr(channel, "name") | |
| else None | |
| ), | |
| member=( | |
| SerializableMember( | |
| member.id, | |
| member.name, | |
| member.display_name, | |
| [role.id for role in member.roles], | |
| member.guild.id, | |
| str(member.avatar.url) if member.avatar else None, | |
| str(member.color), | |
| ) | |
| if member | |
| else None | |
| ), |
| print(f"Logged in as {self.user} (ID: {self.user.id})") | ||
|
|
||
| for guild in self.guilds: | ||
| await self.tree.sync(guild=guild) |
There was a problem hiding this comment.
There's some problem with adding the configure command to a new app. I got it to work by adding
self.tree.copy_global_to(guild=guild)| # Check if the message is a direct message | ||
| is_direct_message = isinstance(message.channel, discord.DMChannel) | ||
|
|
||
| # If it's a direct message, we'll handle it differently | ||
| if is_direct_message: |
There was a problem hiding this comment.
| # Check if the message is a direct message | |
| is_direct_message = isinstance(message.channel, discord.DMChannel) | |
| # If it's a direct message, we'll handle it differently | |
| if is_direct_message: | |
| # If it's a direct message, we'll handle it differently | |
| if isinstance(message.channel, discord.DMChannel): |
|
|
||
| logging.basicConfig(level=logging.INFO, format="%(asctime)s:%(levelname)s:%(name)s: %(message)s") | ||
|
|
||
| MAX_MESSAGES = 2 |
| """ | ||
| self.command_queue.put(("process_event", (event, user_info))) | ||
|
|
||
| def _process_event(self, ecosystem: Ecosystem, event_data: tuple[DiscordEvent, UserInfo]) -> None: |
There was a problem hiding this comment.
Normally when you have two methods with the same name except a leading underscore, one of them calls the other and it's just organized that way for internal reasons. These two do different things, though, so they should probably have more descriptive names to distinguish them.
No description provided.