shows results of removing client/mob verbs on client #2484
Conversation
…erb removal more correct
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…lso fix listx.Remove(listx) by copying the argument list before reading
Ruzihm
changed the title
| } | ||
|
|
||
| Appearance = appearance; | ||
| appearanceSystem.RefreshVerbs(); |
There was a problem hiding this comment.
This is causing way too much overhead when connected to a real server. Will need an alternative to this....
There was a problem hiding this comment.
I'm a little unsure what else could be done here. Ideally, we would only update verbs for clients which are gaining or losing executable verbs.
But I'm wondering if it's acceptable to do something simple and just have the client periodically call RefreshVerbs on a timer of some kind...
| private async Task StartVerbRefresher() { | ||
| while (true) { | ||
| await Task.Delay(TimeSpan.FromSeconds(timeToRefreshVerbs)); | ||
| _taskManager.RunOnMainThread(_verbSystem.RefreshVerbs); | ||
| } | ||
| } |
There was a problem hiding this comment.
This works but I'm not sure if this is the best way.
|
Also I noticed that when reading/clearing |
| // In case this is a "listx.Remove(listx)" situation, copy the contents here to avoid modification while enumerating | ||
| // TODO: check for that case first to avoid unnecessary copy? | ||
| var subtraction = argumentList.EnumerateValues().ToList(); |
There was a problem hiding this comment.
This will be a lot less performant with large lists. I would prefer instead doing a Cut() if it's the same list.
| yield return new(verb); | ||
| } | ||
|
|
||
| public override bool ContainsKey(DreamValue value) { |
There was a problem hiding this comment.
This method is a little confusingly named. This looks for a key in the associative array, and an integer can never be one of those.
This should be removed.
| } | ||
| } | ||
|
|
||
| public override bool ContainsKey(DreamValue value) { |
| SubscribeNetworkEvent<FlickEvent>(OnFlick); | ||
| SubscribeLocalEvent<DMISpriteComponent, WorldAABBEvent>(OnWorldAABB); | ||
|
|
||
| _ = StartVerbRefresher(new()); |
There was a problem hiding this comment.
I would prefer this to be a simple _verbSystem.RefreshVerbs() inside the Update() method, so it's a little more in tune to the server's update schedule.
This can also be removed if periodically calling RefreshVerbs() is how we're going to go about it.
2 participants
Closes #2342.
The verb list classes didn't override
ContainsKey,ContainsValue, orRemoveValue, which led to verb removal not working as expected. I also added an explicit verb refresh step to appearance updates on the client side. to ensure that atom verb updates are kept up to date on the client.Also, while testing
listx.Remove(listx)I had an issue where the list was being modified while iterated over so now list.remove will copy lists passed as arguments before iterating over them.Testing code on top of testgame:
Results:
thing1-4remove the mob verbs,thing5-8remove the client verbs.