Improved Multiplayer Support

[Denneisk]

Hi, I was hoping to have this addon on a server but I think there's some points for improvement for it to be better for use on servers. I know this addon is primarily made for singleplayer, but I'd really like for it to be usable in public Sandbox without security/stability concerns because I find this addon really useful.

• [x] Actions should verify the player and permissions with CPPI.
  • For performance, maybe make this a toggle for the server to run this if needed.
• [x] Synchronization may need to be queued to avoid overloading players.
  • Consider using netstream to automate this.
• [x] High amount of network strings should be condensed into a single networkstring and send IDs in the message instead instead.
• [ ] #40
  • I think this one would rewriting the whole thing, so bear with me, it's just a suggestion.
  • If this is redundant please excuse me. I saw #28 but thought it could be improved further.

I know that last one is excessive, but I hope you can at least consider it. I may try to spend some time contributing to some of these issues if you'll allow, but I'm coming into this codebase nearly blind.

[penolakushari]

Yeah ragdoll mover is quite a hazard for multiplayer servers. Some of these considerations would be nice to have, as well as feel free to contribute, though so far I just had couple more things to add like advanced bonemerge entities support + reducing amount of entities for gizmos to just 1. What could be a good way to coordinate working on the features?

As for the issue with rgmSyncClient thing, I did happen to do it the other way with client sending the position + angle of a bone from client to the server when it's selected, after what server calculates the rest on its own. I was thinking that all the movement is serverside, so I let it do the calculations + positioning for gizmos. Although now that you mention it, perhaps having clients do all the work may work well.

[Denneisk]

How do you mean coordinate? The first 3 elements of the list are mostly independent.

[penolakushari]

Yeah, the first 3 shouldn't be too hard to put in on their own, I mostly mean making it so we won't end up working on the same thing, as I've had couple of times before where someone else would work on something that I've had covered on my fork already, probably just an issue with lack of communication. Sorry, didn't happen to work much with other people here, so I'm not too sure on how things go usually.

[Denneisk]

No worries. I'll let you know if I start up something, or even draft a PR.

[penolakushari]

Sure thing. Currently I'm about done with the advanced bonemerge support things, gonna merge it in a day or two at best, also, CPPI is prop protection stuff?

[Denneisk]

Yes, CPPI is a prop protection interface implemented by most prop protection addons. You can look at https://ulyssesmod.net/archive/CPPI_v1-3.pdf for a guide. You just need to check target_entity:CPPICanTool(player, "ragdollmover") permissions on the entity before doing anything.

[penolakushari]

Alight, I think I finished up on some of the fixes that I did want to do for the tool feature-wise, just wanted to clear up some stuff to have a plan for the things you request.

1 - again about the CPPI, would there be a need to see if there's any prop protection thing present before using the CPPICanTool? Although I just googled advdupe2 repo and they do run a check if a function exists, so I guess I just got the answer I needed. Should be clear.

2 - Overloading, it's with the whole addition of .rgm table to players, or the sync functions? I guess putting functions into the rgm module would work ok, and would eliminate a bug related to game not assigning those functions to players sometimes

3 - This one's understandable, but I just wanted to ask, whole point is to just use less message slots as to not pollute the pool, right? As for implementation I guess having some table with message IDs as keys for their functions should do the trick

4 - So far I was planning to reduce the amount of entities that the gizmos use down to 1, (from like, 18) although seems like with passing position data between client and server, I've been getting some weird results if I tried to pass calculated bone positions relative to their parents, though passing their world positions seemed to be with acceptable precision. Ragdoll mover does work primarily with physical objects, and I recall having some troubles when trying to get physical info from clients when I took over working on the tool, so calculations for those happen at server.

Although for drawing, client could do some bone tracking on their end so it'd look smoother rather than lagging behind on a moving entity. As for rest of calculations, unsure, I'll see what I could do when reducing the entity count as probably a bunch of things there could use some clean up, the whole thing with calculating where to put the gizmo entity is quite a mess, but hey, it works!

[Denneisk]

1. Yes, it should be pretty simple, just check if CPPI or ent.CPPICanTool is defined before calling it.
2. The sync functions don't have a limit to the amount of data they send so it's possible to send really huge amounts of data all at once which can cause net buffer overflows.
3. Yes. I counted 40 just in one file which is 1/100 of the current Gmod limit. I know that doesn't sound like a lot but users will run into limits quickly especially with having multiple big addons installed.
4. Not sure about this one as I haven't looked too extensively at it, but having clientside position update would at least help with visual lag. At least, you could try sending updates that are "held down" over the unreliable channel and then when the user unclicks, send it as a normal channel message.

[penolakushari]

Got back for a bit, happened to reduce amount of entities used for the gizmos so it's just one now, otherwise everything works absolutely the same as it did before, for better or worse. It should be possible to make it work without entities at all, and from clientside too ig, although I don't feel like getting on that. Happened to move all the stuff used in entities into 1 lua file where it uses tables instead of entities, it shouldn't cause anything bad, right? Didn't happen to use metatables as I didn't see any specific reason to do so Would that be alright enough to cover 4th thing, at least temporarily? https://github.com/Winded/RagdollMover/commit/57df66d83b928dceb97977779e5fc5fbb21fa8d7

[penolakushari]

1 https://github.com/Winded/RagdollMover/commit/e71f75984f2bd6633f2d6d57b292071724c91dab

Tried it with nadmod prop protection, didn't seem to notice much of a performance hit, although perhaps it may matter under heavier load with stuff like projected lights and a lot of entities. May also not scale well with multiple players, but ig only testing in such circumstances would tell that

[penolakushari]

2 https://github.com/Winded/RagdollMover/commit/66288ca4b54cdd33d0c091c5fb81e3cbac769996 Happened to also use a separate global table to store all the ragdoll mover data in, and put sync function into it. Apparently to make it work not too much data had to be sent between server/client, so I guess net streams would be not neccesary, and since some of the optimizations I've done, I think there's no net messages being called in functions that run each tick.

3 https://github.com/Winded/RagdollMover/commit/33a230745cebad647dbc024ecbc8151db6120bdc Reduced amount of pooled messages from 58 to 4, would that be good enough? Having 3 of the messages for each of the 3 tools, as some of the messages use some specific exposed functions from their tool's UI

Don't think I'd work on the clientside calculations for bones, it should be possible to do, but I'm not quite feeling like doing it. Separated it into another issue so perhaps someone else could handle it, if anyone would be up for that. Keeping 1 entity for the ragdoll mover axis, as otherwise there's a possible situation where some prop could sink under the map and clients would have trouble with "seeing" them, and there's the new gizmo offset function which could move that entity elsewhere to allow to manipulate the sunk prop.

Is the current progress satisfactory?

[Denneisk]

Sorry for the radio silence, have been away from Gmod development. This all sounds great, thank you! Sorry I couldn't be of more assistance.

[penolakushari]

No problem, in the meantime I've also managed to get more features, moving some calculations clientside gave me an idea to select any bone from client through aiming at it in-game through a separate selection mode which massively speeds up posing workflow. Gonna leave further clientside calculation-moving for someone who'd be handling the tool after I put up all the new stuff on workshop