Migration to Harmony #1060

krzychu124 · 2021-02-20T23:11:41Z

PR goals

- Delete `RedirectionFramework` from the project in favor of `Harmony` patching library

- Improve runtime compatibility on Linux and MacOS #817

Remove unused reverse redirections
Check, test and reuse Kian's work done for migrating most of Vehicle patches (partial migration of VehicleAI.StartPathFind into harmony. #943, major-feature-harmony2-migration)
Rewrite reverse redirections to harmony delegates
Rewrite leftover RF redirections to either Harmony prefix, postfix or transpiler

Improvements

Fixed double patch causing overwriting CarAI redirection by TrolleybusAI redirection resulting in:
- ignored Individual driving styles
- ignored penalties for road conditions (studded tires, wet/damaged road)
- possible incorrect max speed of vehicles

= to be continued =

update issues list

Closes #864
Closes #865
Closes #1063

Latest build

You can find it here!

Compatible Node Controller:
https://github.com/kianzarrin/NodeController/releases/tag/V2.2.4

…sed code

kianzarrin · 2021-02-21T19:57:44Z

Delete RedirectionFramework from the project in favor of Harmony patching library

is this the continuation of my initiative #865 ? might worth saying fixes 865

krzychu124 · 2021-02-21T20:26:27Z

Delete RedirectionFramework from the project in favor of Harmony patching library

is this the continuation of my initiative #865 ? might worth saying fixes 865

Yes, the plan is to fix all issues regards to harmony and finally forget about Redirection Framework.
I'll try transpiling wherever it is possible or prefix patch if TM:PE changes are too big and transpiling doesn't make sense(further altering the result wouldn't be really "patchable").

kianzarrin · 2021-02-21T20:30:34Z

that is a load off my shoulders! thanks!

kianzarrin · 2021-02-21T20:43:05Z

we could use reverse patch strategy instead of using static global args struct to pass on arguments
https://discord.com/channels/131466550938042369/361891646742462467/778665308948267048
it is more thread-safe.

But that is too complicated ( I can't understand that code) and if it is hard to understand it is hard to maintain. also not sure if it will be compatible if other people also want to patch the same methods.

krzychu124 · 2021-02-21T21:04:35Z

I'll take a look into it. One thing I found is that on Linux Harmony reverse patches tend to cause issues because it seems that mono runtime on linux is way more aggressive when it comes to method inlining.

I'll go for easiest route to remove Redirection Framework first, then I'll explore potential readability/extensibility improvements.

Personally I think it needs to work the same as before, before we will start improving/refactoring that code.

TLM/TLM/Patch/_VehicleAI/_AmbulanceAI/StartPathFindPatch.cs

breakone9r · 2021-02-22T14:02:03Z

Tested this last night. Seems like a step in the right direction. It solved some of the issues I've been having on Linux.

- Replaced ParkingAI reverse redirections with fast delegates (IPassengerCarAIConnection) - Removed unused redirections from CustomCarAI(will be replaced anyways) - Migrated more than half of all redirection to Harmony (10 types left)

zubozrout · 2021-03-05T13:21:38Z

I've only played for a few hours but tried using TM:PE and loaded the game 4 times and so far so good. This work is awesome, thank you @krzychu124.

… for FullDebug target

…n from CustomPathManager to ExtPathManager

krzychu124 · 2021-03-11T00:37:50Z

Pretty much everything was migrated, removed obsolete RedirectionFramework submodule.
I'll try to figure out why sometimes TM:PE patch is slightly different than stock code. Usually changes were necessary due to additional features. I need to revisit those Kian's questions in linked issues.

Mod still works, Linux users should finally use the mod without any weird crashes or issues.
I haven't found any regression when it comes to functionality, but performance tests are welcomed.

kvakvs

Nice
Only style comments and struct construction (not sure if a performance warning or not)

TLM/TLM/Custom/PathFinding/CustomPathManager.cs

TLM/TLM/Manager/Impl/AdvancedParkingManager.cs

TLM/TLM/Manager/Impl/ExtPathManager.cs

TLM/TLM/Patch/_TransportLineAI/StartPathFindPatch.cs

TLM/TLM/Patch/_VehicleAI/_TrainAI/CheckNextLanePatch.cs

TLM/TLM/Custom/PathFinding/CustomPathManager.cs

kianzarrin · 2021-03-29T18:49:03Z

Closes #865

I am not sure if this closes #865 because, we have introduced a lot of prefixes that return false.

I am not sure if it is possible to add prefix/postfix and run through the original code or how important is it to do so.

TLM/TLM/Patch/_CitizenAI/_TouristAI/GetLocalizedStatusPatch.cs

TLM/TLM/Patch/_CitizenAI/_TouristAI/GetVehicleInfoPatch.cs

TLM/TLM/Patch/_RoadBaseAI/ClickNodeButtonPatch.cs

TLM/TLM/Patch/PatchCommons.cs

TLM/TLM/Patch/_VehicleAI/_PassengerCarAI/GetLocalizedStatusPatch.cs

TLM/TLM/Util/TranspilerUtil.cs

TLM/TLM/Patch/_VehicleAI/_PassengerCarAI/GetLocalizedStatusPatch.cs

…ate calls (PassengerCarAI.GetVehicleInstance)

TLM/TLM/Manager/Impl/ExtPathManager.cs

…rom DebugMenu panel (visible only in debug builds)

…e review

…e review /2

kianzarrin · 2021-04-11T05:57:14Z

@krzychu124 is testing branch equivalent to major-feature-branch-harmony ?

kianzarrin · 2021-04-11T05:57:49Z

when are you merging to master?

krzychu124 · 2021-04-11T19:12:40Z

I'll wait for your changes and merge master to testing then merge testing back to master and release/testing
The goal is to have master for continuous work(sort of changes container) and release/testing for workshop releases (always working version)

DaEgi01 · 2021-04-12T11:45:23Z

so testing = labs?

krzychu124 · 2021-04-12T13:17:14Z

Hmm, maybe "new labs", but I'll remove that branch once merged with master, so there will be master (aka development) and release/test and release/stable for easier change tracking.

flarn2006 · 2021-04-17T23:03:25Z

I'm getting a crash when I try to toggle traffic lights for a junction while editing an intersection in the asset editor. I haven't tried it without Traffic Manager, or even with Traffic Manager on any release other than the one from this thread, but I just thought I'd mention it because it is a part of the game that I'd expect Traffic Manager to have interactions with.

Removed unused reverse redirections.

00c83bc

krzychu124 added enhancement Improve existing feature high priority Affects lots of users technical Tasks that need to be performed in order to improve quality and maintainability COMPATIBILITY Mod (in)compatibility / checker Harmony labels Feb 20, 2021

krzychu124 added this to the 11.6.0 milestone Feb 20, 2021

krzychu124 self-assigned this Feb 20, 2021

krzychu124 added 2 commits February 21, 2021 01:08

Squashed commits from 'major-feature-harmony2-migration', removed unu…

13551fe

…sed code

Reordered methods and fields.

443c0e2

krzychu124 commented Feb 22, 2021

View reviewed changes

TLM/TLM/Patch/_VehicleAI/_AmbulanceAI/StartPathFindPatch.cs Show resolved Hide resolved

krzychu124 added 4 commits February 24, 2021 23:12

- Improved TranspilerUtils

65389d1

- Replaced ParkingAI reverse redirections with fast delegates (IPassengerCarAIConnection) - Removed unused redirections from CustomCarAI(will be replaced anyways) - Migrated more than half of all redirection to Harmony (10 types left)

- added missing GameConnectionManager.cs to VCS

b5f21bc

Migrated most Vehicle patches

86c2a94

Added missing file(wip) to the git.

4ef1b78

krzychu124 mentioned this pull request Mar 3, 2021

Game crash (on Linux) TM:PE V11 STABLE #817

Closed

TrainAI patch migration to Harmony

74f6042

krzychu124 added 6 commits March 7, 2021 04:03

HumanAI, ResidentAI, TouristAI patch migration, added missing imports…

6e847f6

… for FullDebug target

Fixed StackOverflow in ResidentAI patch, moved FindCitizenPathPositio…

7822935

…n from CustomPathManager to ExtPathManager

Pathfinding migration

1fb3c3b

Submodule fix

b210e96

Deleted RedirectionFramework submodule, fixed usings

1cf92e9

RedirectionFramework dll copy cleanup

756fed3

krzychu124 requested review from kvakvs, chameleon-tbn and DaEgi01 March 11, 2021 00:29

krzychu124 added the code cleanup Refactor code, remove old code, improve maintainability label Mar 11, 2021

krzychu124 changed the base branch from master to testing March 11, 2021 01:23

krzychu124 mentioned this pull request Mar 14, 2021

Asset Color Expander: Building color set in ACE are ignored in TMPE CustomGetColor #1063

Closed

kvakvs approved these changes Mar 14, 2021

View reviewed changes

Code readability improvements

ce25819

kianzarrin reviewed Mar 27, 2021

View reviewed changes

TLM/TLM/Custom/PathFinding/CustomPathManager.cs Show resolved Hide resolved

kianzarrin reviewed Mar 29, 2021

View reviewed changes

TLM/TLM/Custom/PathFinding/CustomPathManager.cs Show resolved Hide resolved

kianzarrin requested changes Mar 30, 2021

View reviewed changes

krzychu124 added 2 commits April 8, 2021 21:49

Moved IGameConnectionManager outside of api project

189b7b6

Changes after code review, replaced harmonyDelegate with normal deleg…

f2984f5

…ate calls (PassengerCarAI.GetVehicleInstance)

DaEgi01 approved these changes Apr 8, 2021

View reviewed changes

krzychu124 added 4 commits April 9, 2021 00:09

Removed interface abstraction from GameConnectionManager

16e99ff

GetLocalizedStatusPatch - fix typo, removed redundant Update() call f…

f1f98de

…rom DebugMenu panel (visible only in debug builds)

FindCitizenPathPosition() accessibility change - suggestion after cod…

47f0ba3

…e review

FindCitizenPathPosition() accessibility change - suggestion after cod…

4eb6438

…e review /2

kianzarrin approved these changes Apr 10, 2021

View reviewed changes

krzychu124 merged commit 9f7bb61 into testing Apr 10, 2021

krzychu124 deleted the improvement/full-harmony-migration branch April 11, 2021 19:12

originalfoo mentioned this pull request Dec 20, 2021

patch spawn/despawn vehicle using new harmony #864

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migration to Harmony #1060

Migration to Harmony #1060

krzychu124 commented Feb 20, 2021 •

edited by kianzarrin

Loading

kianzarrin commented Feb 21, 2021 •

edited

Loading

krzychu124 commented Feb 21, 2021

kianzarrin commented Feb 21, 2021

kianzarrin commented Feb 21, 2021

krzychu124 commented Feb 21, 2021

breakone9r commented Feb 22, 2021

zubozrout commented Mar 5, 2021

krzychu124 commented Mar 11, 2021

kvakvs left a comment

kianzarrin commented Mar 29, 2021 •

edited

Loading

kianzarrin commented Apr 11, 2021

kianzarrin commented Apr 11, 2021

krzychu124 commented Apr 11, 2021

DaEgi01 commented Apr 12, 2021

krzychu124 commented Apr 12, 2021

flarn2006 commented Apr 17, 2021

Migration to Harmony #1060

Migration to Harmony #1060

Conversation

krzychu124 commented Feb 20, 2021 • edited by kianzarrin Loading

PR goals

- Delete RedirectionFramework from the project in favor of Harmony patching library

- Improve runtime compatibility on Linux and MacOS #817

Improvements

Latest build

kianzarrin commented Feb 21, 2021 • edited Loading

krzychu124 commented Feb 21, 2021

kianzarrin commented Feb 21, 2021

kianzarrin commented Feb 21, 2021

krzychu124 commented Feb 21, 2021

breakone9r commented Feb 22, 2021

zubozrout commented Mar 5, 2021

krzychu124 commented Mar 11, 2021

kvakvs left a comment

Choose a reason for hiding this comment

kianzarrin commented Mar 29, 2021 • edited Loading

kianzarrin commented Apr 11, 2021

kianzarrin commented Apr 11, 2021

krzychu124 commented Apr 11, 2021

DaEgi01 commented Apr 12, 2021

krzychu124 commented Apr 12, 2021

flarn2006 commented Apr 17, 2021

krzychu124 commented Feb 20, 2021 •

edited by kianzarrin

Loading

- Delete `RedirectionFramework` from the project in favor of `Harmony` patching library

kianzarrin commented Feb 21, 2021 •

edited

Loading

kianzarrin commented Mar 29, 2021 •

edited

Loading