patch spawn/despawn vehicle using new harmony

[kianzarrin]

this code is commented out:

TMPE/TLM/TLM/Patch/_Vehicle/SpawnPatch.cs

Line 12 in 37b71da

// TODO this does currently not work with Harmony v1.1

TMPE/TLM/TLM/Patch/_Vehicle/UnspawnPatch.cs

Line 12 in 37b71da

// TODO this does currently not work with Harmony v1.1

Once I moved to harmony 2 should we uncomment this?

VehicleStateManager no longer exists but we can call OnSpawnVehicle(); instead. that method is already being called from CustomVehicle.CustomSpawn() so maybe I should delete Vehicle.Spawn/DespawnPatch

[kianzarrin]

if CustomSpawn is copy of the original method then we we should replace CustomSpawn with the harmony patch.

[kianzarrin]

its a bit hard to compare the code because of the differences in decompiler naming and spacing style.
does anyone know a solution?

[pcfantasy]

@kianzarrin

No quick way.

I think we have to read carefully about these codes and find out the difference.

[kianzarrin]

But the original code was also copy pasted from decompiler why should it be so hard to compare decompiler code with decompiler code?

There is a shit ton of code out there so any automation we develop could be useful and reduce the risk of human error

[kianzarrin]

Maybe we can look at decompiler tmpe code using same decompiler. That should solve almost all problems.

EDIT: Using this technique its becomes easily obvious that custom spawn only calls OnSpawn afterwards.

Its important to compare to older CS dlls if any other differences were found.

[originalfoo]

Seems I spotted that some time ago: #462

I think it would be very useful for TMPE to know when vehicles spawn/unspawn (no idea which is best approach for that).

For example, if we eventually start reserving parking spaces for vehicles, we will need to know if they despawn unexpectedly so we can un-reserve their parking space.

We could also maybe detect many vehicles despawning at same location = likely broken node bug (or at least we could stack trace to get some idea of the code path that led to despawning).

[kianzarrin]

@aubergine10 we already receive notification when vehicle spawn/despawns using redirection. I am saying we are now able to do it with harmony

[originalfoo]

Yup, it looks like we can do it harmony, but note there's two lines of custom code there:

I assume we'd just patch the vanilla .Spawn()?

[kianzarrin]

@aubergine10 There is only one line of custom code (functionally speaking) the method is recursive so where the original code calls Spawn, the redirected method calls custom spawn. so its equivalent.

All we need to do is a post fix (which is what I suspect from almost every custom AI)

EDIT: It is going to feel really good deleting a shit ton of code. maybe keep the original as text since the code is easier to read than the de-compiler?

[originalfoo]

I don't think we should keep the original tbh, as it will go stale if base game gets updated.

Hopefully if we're just doing postfixes on .Spawn() methods we'll never even need to look at that vanilla code again.

[kianzarrin]

then why do we keep stockPathFind.cs ? is it particularly important?

<AdditionalFiles Include="Custom\PathFinding\StockPathFind.cs" />

[originalfoo]

Re: StockPathFind.cs: We keep it so we can do diffs when new game version is released.

BTW, I looking at the code in screenshots above, note that there are two different call contexts:

• CustomSpawn() is called for vehicle and each trailer (if applicable) - ie. it can be called more than once per invocation of parent method
• OnSpawnVehicle() is called only once, regardless of how many trailers

So I think we need two postfixes? One for the vanilla .Spawn() and one for whatever the parent method is of the code in the screenshots above.

[kianzarrin]

@aubergine10 no. only one change (unless if I am making a mistake)

CustomSpawn() is called for vehicle and each trailer (if applicable) - ie. it can be called more than once per invocation of parent method

Note that on the left there is an equivalent recursive call for vehicle and each trailer (if applicable) - ie. it can be called more than once per invocation of parent method. So this line provides no new logic and does not need to be patched. Actually It would have made no difference if this line called Spawn() instead of CustomSpawn() considering Spawn is redirected to CustomSpawn()anyways.

[originalfoo]

Based on the current way the code works:

Let say vehicle has 2 trailers. That's 3 times CustomSpawn() that will be called (once for front vehicle, then once for each trailer). After that "spawn loop" completes, the OnSpawnVehicle() is called once.

So I don't see how we can replicate that with only one patch around the vanilla .Spawn() method, because that method won't know if trailers need adding.

Unless the plan is to call OnSpawnVehicle() multiple times, once for each 'part' of the vehicle, then we will need two postfixes.

The current way it works, OnSpawnVehicle() is only called once, after all parts of the vehicle (front + trailers, if applicable) have been spawned.

[originalfoo]

More clarity: If there is a truck with 2 trailers attached, TMPE will currently see:

CustomSpawn() // truck
CustomSpawn() // trailer 1
CustomSpawn() // trailer 2
OnSpawnVehicle() // only called after all parts of the vehicle are spawned

[kianzarrin]

More clarity: If there is a truck with 2 trailers attached, TMPE will currently see:
CustomSpawn() // truck
CustomSpawn() // trailer 1
CustomSpawn() // trailer 2
OnSpawnVehicle() // only called after all parts of the vehicle are spawned

I think you are wrong. You are confused about how recursive calls work. in the custom code OnSpawnVehicle is being called multiple times.

CustomSpawn() // truck
CustomSpawn() // trailer 1
OnSpawnVehicle() // trailer 1
CustomSpawn() // trailer 2
OnSpawnVehicle() // trailer 2
OnSpawnVehicle() // truck

EDIT: This is what happens if the truck has 4 trailers:
https://youtu.be/nbleC0-YJ88?t=1603

[originalfoo]

Ah, the parent method is CustomSpawn(). Ok, that makes more sense.

Bah, there's actually two loops that contain recursive spawn. The more I look at the vanilla Spawn() method, the more crazy it becomes. Specifically, does it actually need to be recursive given how the game actually works?

As far as I know, there's never anything that has side-vehicles or vertical trailers. Assuming it's always a start vehicle followed by zero or more trailers, it should be possible to just to create and configure those vehicles with a few simple iterations without going recursive and thus reduce call stack and gc.

[originalfoo]

Ok, so there are some wierd vehicle configs that require that extra code in the vanilla Spawn() methods.

So yeah, there's no need for TMPE to replicate those methods, just postfix the event. Earlier I didn't realise it was recursive method (snippets in images didn't show what the parent method was) hence confusion.

[originalfoo]

Considering #921 and #1060 are merged, I think this #864 can be closed?

[kianzarrin]

damn I fixed this ages ago!

[kianzarrin]

TMPE is now harmony only.