search

BUTR / MnB2-Bannerlord-CommunityPatch

This is going to be a mod that just fixes up some things in Mount & Blade 2: Bannerlord before the Devs & QA team can get to them. They have priorities and a process.
MIT License
88 stars 30 forks source link

Steward perk "Bannerlord" not being applied #139

Open Sons-of-Man opened 4 years ago

Sons-of-Man commented 4 years ago

Steward perk "Bannerlord" still not working for me for some reason. Is there any incompatibilities that should be mentioned or load order? Can it be used on a existing save? I am using Bannerlord Tweaks mod, CITEditor and a few others that I am certain aren't causing any problems. Any help is appreciated, thanks!

Skau commented 4 years ago

It should be working. What version are you on? It can be used on an existing save.

Just checked out Bannerlord Tweaks' source code and they are patching the same function that we do for fixing the following perks: Bannerlord, ManAtArms and SwordsAsTribute.

We don't apply our patches to a function if another mod does, so we don't break them. In this case Bannerlord Tweaks "wins", and our patches don't get through. At the moment you would have to disable Bannerlord Tweaks to make them work. We are looking into this to find a good solution.

Sons-of-Man commented 4 years ago

It should be working. What version are you on?

Beta branch 1.1.0. Saw no huge need too update too the next. Could one of those tiny fixes/reversions possibly break this?

Skau commented 4 years ago

Look at the edit on my last comment.

Sons-of-Man commented 4 years ago

Ah alrighty, been clueless for the past couple hours fiddling around trying to find out what was causing it. Would have never guessed this too be the reason but had a small feeling Bannerlord Tweaks may have been overwriting so I moved Community Patch towards the end of the load order, since that didn't fix it, here I am lol. Tough choice too choose between the two!

Tyler-IN commented 4 years ago

Load order won't matter for this - the patches are applied after campaign data is loaded.

We'd somehow need BL tweaks or other mods to apply the patches after we do if they want to patch the same methods. Our mod is expecting the default broken behavior from the exact code we know needs to be patched.

We're patching just to make fixes, they're patching to change behavior, so our policy since the beginning is don't interfere with non-default behavior... e.g. If some other mod patches a broken function instead of using it, it could also be trying to fix it or accounts for it being broken and we don't want to interfere with what the other mod expects and cause it to break (e.g. double fixing it, doubling some number or causing general wackiness).

If they patch after we've patched it, they'll know what they're getting. They can query the method via Harmony to see that we patched it.

We'd need to expose some sort of events around when we patch and then make a PR for BL Tweaks to have it detect our mod being loaded and patch after our patches are applied. edit; Or they could just patch after our mod does by some other means, like moving their PatchAll call to OnGameInitializationFinished. The module order will actually have an effect there.

This needs some more investigation and we'd like feedback from the authors of the affected mods - if they want to just copy our code, copy our fixes into their mods that works fine and it's less work for us - we're fully open source and MIT licensed, they don't even need to mention it really unless they copy basically the whole mod - otherwise we need to coordinate, or the other mods can be careful what they patch and test their compatibility with our patches.

Skau commented 4 years ago

@Sons-of-Man Could you try disabling PartySizeTweak in the Bannerlord Tweak settings and see if it works then?

Sons-of-Man commented 4 years ago

@Sons-of-Man Could you try disabling PartySizeTweak in the Bannerlord Tweak settings and see if it works then?

Can confirm this has fixed it, but would it be better off disabling the whole mod since it will just overwrite constantly? Especially if you guys fix other perks. Cause I am not entirely sure what else perk wise isn't working, least that I have noticed yet lol.

Skau commented 4 years ago

Great. No, it's fine, they don't patch at all when the whole module like that is disabled. If you had it enabled, but instead had either of the sub settings, LeadershipPartySizeBonus or StewardPartySizeBonus (or none of them for that matter), our patch wouldn't get through.

Skau commented 4 years ago

I'm not about to go through all their source code though and see what affects our fixes. So do it if you want. If you find any more issues, just let us know. Or just disable either Bannerlord Tweaks or Community Patch entirely.

Tyler-IN commented 4 years ago

Pinging @mipen from https://github.com/mipen/BannerlordTweaks