Fix issue #418: Invoke the code to check and apply a fee on group creation if configured. #461
Conversation
| IMoneyModule mm = scene.RequestModuleInterface<IMoneyModule>(); | ||
| if (mm == null) | ||
| { | ||
| remoteClient.SendAgentAlertMessage("Server configuration error - cannot create group without money module.", false); |
There was a problem hiding this comment.
This introduces a new semantic: previously the absence of a money module still allowed group creation. I suggest restoring that semantic.
There was a problem hiding this comment.
While it is true the absence of the money module allowed it previously (on this one operation), that is because this one operation was completely missing all money-related code/checks. (Sending an IM to a user also succeeds with no money module because of course that operation is currently free...)
As far as I understand things, the money module is not optional, there are many places that would crash or fail if one was not present. (One example discovered below is that mesh uploads will crash.)
More importantly, this suggestion would make it inconsistent with the application of the money balance checks used on other operations (e.g. in HandleUDPUploadRequest and NewAgentInventoryRequest which both have code to return an error to the user if there is no money module. Clearly the intention is to fail an operation that has a fee if there is no money module available.
However, I notice that NewAgentInventoryRequest (the mesh upload case) will crash with a null reference exception just before it attempts to report the insufficient funds if there's no money module defined. That said, crashing before performing the operation still means it won't succeed when there is no money module.
I modeled this change after the other cases, thus it has the same failure result if there's a fee and no money module installed. I don't think we should be changing that behavior for this specific new check, just because there wasn't any check at all previously.
There was a problem hiding this comment.
That makes sense then: always helps to see the larger picture.
| { | ||
| mm.ApplyGroupCreationCharge(remoteClient.AgentId); | ||
| remoteClient.SendCreateGroupReply(groupID, true, "Group created successfullly"); | ||
| } |
There was a problem hiding this comment.
No error to the user if the group failed to be created?
There was a problem hiding this comment.
Good point. I improved the previous behavior, which was to always report the group creation as a success, even when it failed, by making that result conditional on actual success. I didn't add a new message for the failure case, and maybe that would be a good improvement here.
|
I'm going to add an error response to the user in the event of a group creation failures. I'm also going to take a look at skipping all of this code (first comment) if the operation is free. |
|
Unfortunately it's not possible to precheck if a fee applies to group creation because it is the money module that owns that data (that's why we're trying to get a reference to the money module here in the first place, to do that check). However, I've added the user message for the failure case (and fixed the spelling error in the success case!). |
|
I'm going to fix that mesh upload null references as part of this PR since it's basically the same code. One more commit coming. |
|
Okay, I think that's it. Good to go? |
The code to check and apply a fee on group create has never been invoked. This commit updates the group creation (when using FlexiGroups) to check and apply if configured with
PriceGroupCreatein theHalcyon.inifile.