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
1 Jan 14, 2022
1 Feb 13, 2022
3 Jun 08, 2021
0 Jun 29, 2022
2 Jan 27, 2022
2 Jun 06, 2022
14.2k Jan 03, 2023
2 Jan 08, 2022
3 May 13, 2022
4 Dec 28, 2021
32 Nov 13, 2022
6 Jul 14, 2022
159 Dec 09, 2022
3 Feb 03, 2022
8 Nov 22, 2022
11 Oct 24, 2022
27 Nov 01, 2022
6 Jun 04, 2022
884 Dec 25, 2022
2 Sep 23, 2022