Ship trading & builtby

[Demolish50]

Ship trading needs to also change the builtby attribute. If you don't, when someone removes their owned block it removed all the blocks on the ship that was traded.

[jpcsupplies]

Can you elaborate more? The builtby shouldnt effect the ownedby ?

[Demolish50]

Every block you build, even armor blocks have a builtby node that contains the playerid of the player who built it. It's how the in game block limit system keeps track of who built what blocks.

[Demolish50]

It seems you are only transferring functional computer blocks when doing a ship trade.

Here is a cubeblock that is an armor block

`

LargeBlockArmorBlock 214531349561733432 ` So this is problematic for a few reasons. 1. If remote block removal is enabled, after a trade, the original owner can simply remove all the armor blocks if he chose to. 2. Even if he doesnt all those blocks still count against his total allowance of built blocks.

[midspace]

Agreed. Not changing the BuiltBy is dangerous.

Sample code for changing it is located here. https://github.com/midspace/Space-Engineers-Admin-script-mod/blob/master/midspace%20admin%20helper/Data/Scripts/midspace.adminscripts/Messages/Sync/MessageSyncGridChange.cs#L242

[jpcsupplies]

also not sure if related just had a crash after using sell ship buyship and bal: 2017-03-26 16:17:21.123 - Thread: 1 -> Exception occured: System.InvalidOperationException: Collection was modified; enumeration operation may not execute. at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource) at System.Collections.Generic.Dictionary2.KeyCollection.Enumerator.MoveNext() at Sandbox.Game.Entities.Cube.MyCubeGridOwnershipManager.RecalculateOwners() at Sandbox.Game.Entities.MyCubeGrid.DoLazyUpdates() at Sandbox.Game.Entities.MyCubeGrid.UpdateAfterSimulation() at Sandbox.Game.Entities.MyEntities.<UpdateAfterSimulation>b__d(MyEntity x) at VRage.Collections.MyDistributedUpdater2.Iterate(Action`1 p) at Sandbox.Game.Entities.MyEntities.UpdateAfterSimulation() at Sandbox.Game.World.MySector.UpdateAfterSimulation() at Sandbox.Game.World.MySession.UpdateComponents() at Sandbox.Game.World.MySession.Update(MyTimeSpan updateTime) at Sandbox.MySandboxGame.Update() at Sandbox.Engine.Platform.Game.UpdateInternal() at Sandbox.Engine.Platform.Game.RunSingleFrame() at Sandbox.Engine.Platform.FixedLoop.<>cDisplayClass1.b0() at Sandbox.Engine.Platform.GenericLoop.Run(VoidAction tickCallback) at Sandbox.Engine.Platform.Game.RunLoop() at Sandbox.MySandboxGame.Run(Boolean customRenderLoop, Action disposeSplashScreen) at VRage.Dedicated.DedicatedServer.RunInternal() at VRage.Dedicated.DedicatedServer.RunMain(String instanceName, String customPath, Boolean isService, Boolean showConsole) at VRage.Dedicated.DedicatedServer.ProcessArgs(String[] args) at VRage.Dedicated.DedicatedServer.Run[T](String[] args) at SpaceEngineersDedicated.MyProgram.Main(String[] args) 2017-03-26 16:17:21.127 - Thread: 1 -> Hiding window

[jpcsupplies]

So looks like we need to change https://github.com/jpcsupplies/Economy_mod/blob/master/Economy/Data/Scripts/Economy.scripts/Messages/MessageShipSale.cs#L248 to also do ((MyCubeGrid)grid).TransferBlocksBuiltByID(identity.IdentityId, selectedPlayer.IdentityId);

Since non owned blocks also have a built by, we also need to rework the code that skips over armor blocks etc, and change that too.. ?

I am going to post a recommendation on the Frontier Economy page recommending in the short term that admins disable the remote removal of owned blocks option. Is that an option admin mod can access? Shared hosting servers may struggle to access an option they can only touch in XML.

[midspace]

Can some explain to me what the loop starting here does? https://github.com/jpcsupplies/Economy_mod/blob/master/Economy/Data/Scripts/Economy.scripts/Messages/MessageShipSale.cs#L254

Because as far as I understand, ownership was changed at L250. There are lots of calls to fetch the block definition using GetObjectBuilder(), which is a very costly operation, and then nothing uses the definition.

@temar96

[temar96]

your right i dont see a purpose in those 2 lines, it may have been used for something earlier on in the coding

[temar96]

the loop changes ownership of the individual blocks that have ownership but the blockDefintion variable doesnt seem to be used

[jpcsupplies]

So far testers seem to indicate other than a brief client residue which goes away when they reconnect the issue seems to resolved by your last commit.

[midspace]

Could you elaborate on what client residue is?

[jpcsupplies]

when ownership changes, the local client still see's it as not changed.. so its like "residual" traces of the prior ownership. If they reconnect both client and server see the new owner fine.

[jpcsupplies]

AFAIK this is now all working correctly