This isn't quite about a bug or the like. Think of it like a subissue of #3, but it's just a design discussion that have to do with the merge:tm:.
As of right now, molter (essentially) analyzes parameters (for its own use) upon function decoration. This may seem sane, but there is one issue: dealing with self parameters (or rather finding out when they're there and ignoring them if they are).
self isn't exactly a special variable in Python - it can be named anything, and isn't typehinted or marked as anything special. We want to avoid it if it is there (it's always possible to declare a command in __main__, and is intended behavior), but... well, we can't. Not really.
molter does the next best thing and checks if the function is "nested" - if it's in something, classes included. Based on that, we can theoretically ignore self if it is. However, this method uses a hack and is a hack, having many cases where this would fail. As such, another method is likely needed. Maybe.
There aren't any better ways until molter is merged, but once it is...
Solution 1
Analyze the function as it is being added to the bot's dicts of prefix commands. As this is done with the context needed to determine if the command is in a scale (or in the bot class) or not, this is possible.
The best way to do this is likely to add an extra argument to add_message_command that indicates if the underlying function has a self statement or not (defaulting to it not), and then running the parameter analyzing in the function itself. This also allows people adding their own commands manually to have some sort of control over that process, too.
Basically:
def add_message_command(command: MessageCommand, *, has_self: bool = False):
command.parse_parameters(has_self=has_self)
... # rest of logic here
# scales and in-bot commands can simply make has_self True when running that
There is one drawback: if invalid parameters are passed (an invalid converter, etc.), then the user won't found out it's invalid until the bot is loaded. However, this method has the benefit of allowing all associate_converters to be associated before the command is parsed, meaning we don't have to dig into the parsed parameters to replace things as needed.
Solution 2
Analyze all parameters (minus whatever one's the first one) on function decoration. Then, if the command is being loaded in a place that indicates the function has self, edit the parameters list so that the first element is removed.
This is very simple, in like with what dis-snek does, and allows for errors to happen pretty early on. However, this means that associated converters have to do what they currently do and dig into the current parameters to adjust them as needed, which is somewhat ugly. This also makes globally_associate_converter not be able to apply retroactively, and also makes it a pain to do if we do what Silasary mentioned here, as, well, we wouldn't get the bot's associated converters until after the parameters have been parsed and messed around (and so they would have to be messed around with *again).
Conclusion
I'm leaning towards solution 1 personally, but I'm curious to see what everyone else thinks. I also do wonder if it's even worth the pain when the hack works... fine in most cases.
Again, I'm looking for ideas and feedback, so do say your thoughts!
help wanted
3 Feb 27, 2022
13 Dec 15, 2022
27 Dec 30, 2022
1 Jan 04, 2022
0 Jul 26, 2021
218 Dec 25, 2022
1 Oct 25, 2021
30 Nov 29, 2022
326 Jan 03, 2023
4 Mar 13, 2022
12 Sep 29, 2022
153 Dec 28, 2022
5 Nov 27, 2021
66 Dec 13, 2022
32 Dec 30, 2022
9 Mar 06, 2022
20 Dec 30, 2022
313 Jan 08, 2023
7 Jul 04, 2022
8 Oct 24, 2022