features and fixes

[canerksk]

When a container was emptied, the content was not updated.

[cbnolok]

That's becoming a big amalgamation of commits... I'll comment separately each feature, but please try to avoid multiple features in the same PR :)
Just out of curiosity, for container empty, did you try to just use Update()? Btw your code isn't accounting for clients other than the owner viewing that container, also it might show the container even if it wasn't opened, but i might be wrong.

[canerksk]

That's becoming a big amalgamation of commits... I'll comment separately each feature, but please try to avoid multiple features in the same PR :)

Just out of curiosity, for container empty, did you try to just use Update()? Btw your code isn't accounting for clients other than the owner viewing that container, also it might show the container even if it wasn't opened, but i might be wrong.

yes, I tried all of the commands like findlayer.21.empty, update, updatex, character update, updatex, resend, after saying empty, the contents of the container are not updated at all and after saying empty, the content remains without opening the container again.

[canerksk]

Container empty update I may have acted hastily since the recheck was one of the first commits.

[cbnolok]

Thanks for the comment and obviously for the PR! So do you plan to revise that part of the code?
I have more questions/observations:

1. Could you change OF_PetBehaviorOwnerNeutral to work the opposite way? So that people won't need to change the ini file to keep the old behavior they are used to.
2. Also can you add Changelog entries for your changes?
3. Have you tested all the possible situations where CChar::SetDisconnected might be called? If the trigger is meant to be called only for mounted/to be mounted pets, what do you think about adding a check to ensure that?
4. Could you add an ini switch to enable calculations based on COVERAGEARMOR, instead of using that if set by default?
5. What if we want EventsNPCMountable to work also for brain_monster or brain_dragon entities? Could we remove that limit and use it for every ridable char?
6. In TRIGGER_DEPOSIT, we should check if the item is still valid, since someone might call DELETE, maybe even without doing RETURN 1.
7. Is everything reasonably tested? Since we are trying to focus on bug fixing.

[canerksk]

2. Changelog will be added with the latest updates.

3. Since only mounts can be disconnected (logout) from NPC groups, I think it will be enough to specify only m_pNPC here, and since only one trigger works, nothing else does. But it is still set to trigger for the NPC and brain_Animal groups.

4. Actually if the COVERAGEARMOR value is entered it takes it into account, if it is not entered it calculates based on the normal default values. So I am not sure if an ini setting is needed for this.

5. I didn't know that brain_dragon group NPCs were rideable. Of course, I removed the brain animal query.

6.I don't understand what you mean here, return 1 is just a return that prevents the money from being deposited to the bank and causes it_gold to fail to drop together with return 1, if this return is not there, it will continue to work normally. This part has been tested, I don't understand exactly what you mean.

Of course most of them have been tested but I will retest the untested ones and send a commit. Also you are right about the container empty issue, I tried it and even if the container is not open, when you empty it, the container opens, so is there a method in Sphere that we can query whether a container is open or not? I noticed something like this, if a container is not open at all, when it opens, the sound of the container is heard, but if it is already open and you try to open it again, there is no sound, is this a client-side situation?

[cbnolok]

3. I checked every SetDisconnected call, it's also being called when the char is read from the save file (CChar::r_Load), so it's worth adding a !g_Serv.IsLoading() check.
4. My thought is that we can provide a default scriptpack with COVERAGEARMOR set but as a shard owner we might not want to use that feature. Or a more probable scenario is setting COVERAGEARMOR in several scripts, then changing idea and being able to use the previous behavior in no other way than going back to scripts and call COVERAGEARMOR.
5. Imagine having a script with REMOVE in the @deposit trigger (i said delete before but obv it was a misnaming), and the same scripter isn't adding a return 1 or doing anything else to abort the deposit. What will happen?
6. There's the CClient::LogOpenedContainer method, which is called by CClient::addContainerSetup. There's a snippet from another part of the code: auto itContainerFound = client->m_openedContainers.find( pItemCont->GetUID().GetPrivateUID() );

[cbnolok]

Could you add that ini setting?