Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add a command to convert existing local pins to server pins #17

Closed
3 tasks done
Mydayyy opened this issue Mar 7, 2021 · 17 comments
Closed
3 tasks done

Add a command to convert existing local pins to server pins #17

Mydayyy opened this issue Mar 7, 2021 · 17 comments
Labels
Feature New feature or request

Comments

@Mydayyy
Copy link
Owner

Mydayyy commented Mar 7, 2021 •
edited
Loading

  • Create a basic command to upload all existing local pins to the server, no exceptions
  • Create an advanced command to upload all existing pins to the server:
    • Do not upload pins to the server, when the server has a similar pin placed
    • similar pin is decided by the radius alone, other factors (type, text) are ignored
  • Create a command to delete all local pins
@Mydayyy Mydayyy added the Feature New feature or request label Mar 7, 2021
@Mydayyy Mydayyy mentioned this issue Mar 7, 2021
Closed
@ghost
Copy link

ghost commented Mar 7, 2021 •
edited by ghost
Loading

+1 for this suggestion.
Not sure if it's possible, but if the command could detect nearby locally placed pins, to avoid duplicates, that would be even nicer 👍 . So if it detects a nearby local pin, delete the local one and sync up the ones placed on the server.
So e.g. if a player joins the server, but already has their own locally placed markers in or around the same places, he won't get a bunch of duplicates.

@Mydayyy
Copy link
Owner Author

Mydayyy commented Mar 7, 2021

Greetings,

just converting local pins to server pins is pretty trivial and should easily be done.

About the duplicate thing:
It would be possible too, but the question is what exactly identifies a duplicate pin. Which radius? Same type or not? Same text or not? and by that I fear that the user doesn't exactly know what he is getting when typing that command. It might very well just ignore local pins which are not duplicates but still close together. It might be useful to implement 3 commands, e.g:

/convertpins all
/convertpins ignoreduplicates
/deletellocalpins

The first and last command are pretty straight forward, but it will probably result in not everyone being happy with the detection of duplicate pins, so it will probably still require either cleanup of duplicate serverside pins or adding missing serverside pins after running that command.

I am planning to start developing the command feature tomorrow, maybe I'll get an idea. If you have any input Id be happy to hear :)

Cheers
Mydayyy

@ghost
Copy link

ghost commented Mar 7, 2021 •
edited by ghost
Loading

Yeah, I'm not sure how well it would work either 😄

Which radius?

Great question. I have no good answer to this, because as you say, maybe a "close" radius would suit me & my friends, and wouldn't suit others.

Same type or not? Same text or not?

In my group of players, neither of us use the same icons, so that wouldn't work. Same goes for text.

The three commands would probably be a good idea.. Thinking the /deleteAllLocalPins might be the way to go for all players except one, and let the last one /convertpins all.

You could make the "detect duplicate" option optionable, so the default /convertpins would be ignoring the duplicates, and /convertpins removeLocalDupes could be the optional choice.

@JTF195
Copy link

JTF195 commented Mar 7, 2021

For the duplicate prevention, I would say just ignore any pins where the icon is overlapping when fully zoomed in.

Any pins that already exist server-side can be considered authoritative, and text can be fixed manually on the resulting single pin.

Another consideration: It might be wise to add some server-side configuration options to determine who is allowed to add/remove pins from the server. (Mainly important for the convert/import feature, but would be useful in general)

There is already an opt-in on the server and for each user, but I could see vandalism issues popping up on public servers unless an explicit allow list is also implemented.

@JTF195
Copy link

JTF195 commented Mar 7, 2021 •
edited
Loading

Another couple of options to consider:

An option to silently ignore duplicate pins that are 100% overlapping, at all times. This would eliminate duplicates from other mods (such as Locator), which might not be able to use their own duplicate prevention mechanisms if the pins are stored server-side

An option to ignore pins from AutoMapPins altogether, as they are added and removed automatically in bulk and could place unnecessary strain on the server.

@Mydayyy
Copy link
Owner Author

Mydayyy commented Mar 8, 2021 •
edited
Loading

Greetings,

I consolidated the ideas into a list of tasks to get a better overview:

For the last two points I will create a new issue, the first three issues belong to this ticket and I will start working on those.

Cheers
Mydayyy

@Mydayyy Mydayyy added this to the 1.3.0 milestone Mar 9, 2021
@Mydayyy
Copy link
Owner Author

Mydayyy commented Mar 12, 2021 •
edited
Loading

The three commands are implemented, currently living on branch 17-pin-commands. I made the duplicateRadius configurable and it defaults to 15.0.

I'll take a look at other pin mods tomorrow (locator, automappins) and try to work out a sane behavior for ssm.

I hope to be able to test friday/saturday and have a RC ready sometime during the day.

Cheers
Mydayyy

@Mydayyy
Copy link
Owner Author

Mydayyy commented Mar 16, 2021

Pushed an RC2 today, with a small bugfix.

I also changed the command from /convertpins removelocaldupes to /convertpins ignorelocaldupes since I feel this conveys the actual action better. Remove would imply that the local duplicates are being removed by the command when in fact they stay but are just not uploaded to the server.

I didn't encounter any other issues while testing today so I hope I can push that out as a new version tomorrow.

@JTF195
Copy link

JTF195 commented Mar 16, 2021 •
edited
Loading

I get the following errors when connecting to my dedicated server with WeylandMod.Core enabled on the client side
Even with none of the other modules enabled on the client, and nothing but SSM on the server

[Info   : Unity Log] 03/16/2021 04:13:46: Exception in ZRpc::HandlePackage: System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.IO.EndOfStreamException: Unable to read beyond the end of the stream.
  at System.IO.MemoryStream.InternalReadInt32 () [0x00034] in <eae584ce26bc40229c1b1aa476bfa589>:0
  at (wrapper remoting-invoke-with-check) System.IO.MemoryStream.InternalReadInt32()
  at System.IO.BinaryReader.ReadInt32 () [0x00015] in <eae584ce26bc40229c1b1aa476bfa589>:0
  at ZPackage.ReadByteArray () [0x00000] in <f48d6a22696245bf8d820aad6afa4fdb>:0
  at WeylandMod.Core.CorePlugin.ReadServerConfig (ZPackage pkg) [0x00000] in <5d99a99090024abab84c5c7864ac3b24>:0
  at MonoMod.Cil.RuntimeILReferenceBag+FastDelegateInvokers.Invoke[T1] (T1 arg1, MonoMod.Cil.RuntimeILReferenceBag+FastDelegateInvokers+Action`1[T1] del) [0x00000] in <9218caa36358471fb3c33e4432ee554c>:0
  at (wrapper dynamic-method) ZNet.DMD<ZNet::RPC_PeerInfo>(ZNet,ZRpc,ZPackage)
  at (wrapper managed-to-native) System.Reflection.MonoMethod.InternalInvoke(System.Reflection.MonoMethod,object,object[],System.Exception&)
  at System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00032] in <eae584ce26bc40229c1b1aa476bfa589>:0
   --- End of inner exception stack trace ---
  at System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00048] in <eae584ce26bc40229c1b1aa476bfa589>:0
  at System.Reflection.MethodBase.Invoke (System.Object obj, System.Object[] parameters) [0x00000] in <eae584ce26bc40229c1b1aa476bfa589>:0
  at System.Delegate.DynamicInvokeImpl (System.Object[] args) [0x000e7] in <eae584ce26bc40229c1b1aa476bfa589>:0
  at System.MulticastDelegate.DynamicInvokeImpl (System.Object[] args) [0x00008] in <eae584ce26bc40229c1b1aa476bfa589>:0
  at System.Delegate.DynamicInvoke (System.Object[] args) [0x00000] in <eae584ce26bc40229c1b1aa476bfa589>:0
  at ZRpc+RpcMethod`1[T].Invoke (ZRpc rpc, ZPackage pkg) [0x0001d] in <f48d6a22696245bf8d820aad6afa4fdb>:0
  at ZRpc.HandlePackage (ZPackage package) [0x00042] in <f48d6a22696245bf8d820aad6afa4fdb>:0
  at ZRpc.Update (System.Single dt) [0x0001e] in <f48d6a22696245bf8d820aad6afa4fdb>:0

[Info   : Unity Log] 03/16/2021 04:13:46: Exception in ZRpc::HandlePackage: System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.NullReferenceException: Object reference not set to an instance of an object
  at ServerSideMap.InitialMapSync.SendChunkToServer (ZRpc client, System.Int32 chunk) [0x0007d] in <846274841e524258b727a23b8119320a>:0
  at ServerSideMap.InitialMapSync.OnReceiveMapDataInitial (ZRpc client, ZPackage mapData) [0x00089] in <846274841e524258b727a23b8119320a>:0
  at (wrapper managed-to-native) System.Reflection.MonoMethod.InternalInvoke(System.Reflection.MonoMethod,object,object[],System.Exception&)
  at System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00032] in <eae584ce26bc40229c1b1aa476bfa589>:0
   --- End of inner exception stack trace ---
  at System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00048] in <eae584ce26bc40229c1b1aa476bfa589>:0
  at System.Reflection.MethodBase.Invoke (System.Object obj, System.Object[] parameters) [0x00000] in <eae584ce26bc40229c1b1aa476bfa589>:0
  at System.Delegate.DynamicInvokeImpl (System.Object[] args) [0x000e7] in <eae584ce26bc40229c1b1aa476bfa589>:0
  at System.MulticastDelegate.DynamicInvokeImpl (System.Object[] args) [0x00008] in <eae584ce26bc40229c1b1aa476bfa589>:0
  at System.Delegate.DynamicInvoke (System.Object[] args) [0x00000] in <eae584ce26bc40229c1b1aa476bfa589>:0
  at ZRpc+RpcMethod`1[T].Invoke (ZRpc rpc, ZPackage pkg) [0x0001d] in <f48d6a22696245bf8d820aad6afa4fdb>:0
  at ZRpc.HandlePackage (ZPackage package) [0x00042] in <f48d6a22696245bf8d820aad6afa4fdb>:0
  at ZRpc.Update (System.Single dt) [0x0001e] in <f48d6a22696245bf8d820aad6afa4fdb>:0

[Info   :ServerSideMap] Client received initial pin data by server. Pins: 0
[Info   :ServerSideMap] ClientAppendPins 0
[Info   : Unity Log] 03/16/2021 04:13:46: Session auth respons callback

[Info   :ServerSideMap] ClientAppendPins 0

This doesn't seem to happen when loading into a world locally and switching between characters. The map syncs, pins sync, etc

@Mydayyy
Copy link
Owner Author

Mydayyy commented Mar 16, 2021

Even with none of the other modules enabled on the client, and nothing but SSM on the server

Do you mean absolutely no plugins on the client and only SSM on the server?

Is that something that's happening now with RC builds or with the official release builds?

Cheers
Mydayyy

@JTF195
Copy link

JTF195 commented Mar 16, 2021 •
edited
Loading

It happens with rc2. I haven't tested release builds.

SSM on the dedicated server by itself

SSM, WeylandMod.Core and MMHOOK_assembly_valheim on the client
(with ChangeVersion = false in the WeylandMod.Core config file)

  • with or without any of the other WeylandMod plugins,
  • with or without any other mods

Any other combination of mods without WeylandMod seems to work fine

@Mydayyy
Copy link
Owner Author

Mydayyy commented Mar 16, 2021 •
edited
Loading

Greetings,

Yes, I can reproduce that error, but I also discovered the following:

When running no SSM at all, I still experience a crash when using no serverplugins and only WeylandCore on the client.
Can you confirm that? Given that I would say this is not a SSM issue.

03/16/2021 21:38:29: Exception in ZRpc::HandlePackage: System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.IO.EndOfStreamException: Unable to read beyond the end of the stream.
  at System.IO.MemoryStream.InternalReadInt32 () [0x00034] in <eae584ce26bc40229c1b1aa476bfa589>:0 
  at (wrapper remoting-invoke-with-check) System.IO.MemoryStream.InternalReadInt32()
  at System.IO.BinaryReader.ReadInt32 () [0x00015] in <eae584ce26bc40229c1b1aa476bfa589>:0 
  at ZPackage.ReadByteArray () [0x00000] in <dd568c2b4978432db77fc5f7f770f52b>:0 
  at WeylandMod.Core.CorePlugin.ReadServerConfig (ZPackage pkg) [0x00000] in <5d99a99090024abab84c5c7864ac3b24>:0 
  at MonoMod.Cil.RuntimeILReferenceBag+FastDelegateInvokers.Invoke[T1] (T1 arg1, MonoMod.Cil.RuntimeILReferenceBag+FastDelegateInvokers+Action`1[T1] del) [0x00000] in <e54299200d7d4726aec081e4ca77de7e>:0 
  at (wrapper dynamic-method) ZNet.DMD<ZNet::RPC_PeerInfo>(ZNet,ZRpc,ZPackage)
  at (wrapper managed-to-native) System.Reflection.MonoMethod.InternalInvoke(System.Reflection.MonoMethod,object,object[],System.Exception&)
  at System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00032] in <eae584ce26bc40229c1b1aa476bfa589>:0 
   --- End of inner exception stack trace ---
  at System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00048] in <eae584ce26bc40229c1b1aa476bfa589>:0 
  at System.Reflection.MethodBase.Invoke (System.Object obj, System.Object[] parameters) [0x00000] in <eae584ce26bc40229c1b1aa476bfa589>:0 
  at System.Delegate.DynamicInvokeImpl (System.Object[] args) [0x000e7] in <eae584ce26bc40229c1b1aa476bfa589>:0 
  at System.MulticastDelegate.DynamicInvokeImpl (System.Object[] args) [0x00008] in <eae584ce26bc40229c1b1aa476bfa589>:0 
  at System.Delegate.DynamicInvoke (System.Object[] args) [0x00000] in <eae584ce26bc40229c1b1aa476bfa589>:0 
  at ZRpc+RpcMethod`1[T].Invoke (ZRpc rpc, ZPackage pkg) [0x0001d] in <dd568c2b4978432db77fc5f7f770f52b>:0 
  at ZRpc.HandlePackage (ZPackage package) [0x00042] in <dd568c2b4978432db77fc5f7f770f52b>:0 
  at ZRpc.Update (System.Single dt) [0x0001e] in <dd568c2b4978432db77fc5f7f770f52b>:0

(Logfile with naked server and only weylandCore clientside)

Cheers
Mydayyy

@JTF195
Copy link

JTF195 commented Mar 17, 2021 •
edited
Loading

I was able to reproduce the error under those conditions as well.

It does indeed appear to be an issue with WeylandMod itself.

@JTF195
Copy link

JTF195 commented Mar 19, 2021

When using /deletealllocalpins, I get the following error:

[Error  : Unity Log] InvalidOperationException: Collection was modified; enumeration operation may not execute.
Stack trace:
System.ThrowHelper.ThrowInvalidOperationException (System.ExceptionResource resource) (at <eae584ce26bc40229c1b1aa476bfa589>:0)
System.Collections.Generic.List`1+Enumerator[T].MoveNextRare () (at <eae584ce26bc40229c1b1aa476bfa589>:0)
System.Collections.Generic.List`1+Enumerator[T].MoveNext () (at <eae584ce26bc40229c1b1aa476bfa589>:0)
ServerSideMap.UtilityPin.DeleteLocalPins () (at <846274841e524258b727a23b8119320a>:0)
ServerSideMap.CommandsPinUpsync+ChatPatchInputText.Prefix (Chat __instance, UnityEngine.UI.InputField ___m_input) (at <846274841e524258b727a23b8119320a>:0)
(wrapper dynamic-method) Chat.DMD<Chat::InputText>(Chat)
Chat.Update () (at <f48d6a22696245bf8d820aad6afa4fdb>:0)

Tested under a bunch of different conditions. It will delete one pin and then throw the error.

Running the command subsequent times will continue to delete one pin at a time.

@Mydayyy
Copy link
Owner Author

Mydayyy commented Mar 19, 2021

Greetings,

are you running RC2? I think I addressed this already. This should only happen in RC1

Cheers
Mydayyy

@JTF195
Copy link

JTF195 commented Mar 19, 2021

Yep. RC2

@Mydayyy
Copy link
Owner Author

Mydayyy commented Mar 19, 2021

Oh, I fixed a similar issue in RC2.

I just uploaded RC3 which addresses this: https://github.com/Mydayyy/Valheim-ServerSideMap/archive/rc3-1.3.0.zip

@Mydayyy Mydayyy closed this as completed Apr 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@JTF195 @Mydayyy