search

mipen / BannerlordTweaks

MIT License
59 stars 43 forks source link

Fix for 1.3 #75

Closed joshimoo closed 4 years ago

joshimoo commented 4 years ago

This is for 1.3 Beta players

wfswanepoel commented 4 years ago

Like that you fixed it but your PR is misleading, you have added a new feature within the fix. Would be better to have the fix and the new added feature separate. Adding new features to fix branches might lead to new bugs and issues.

joshimoo commented 4 years ago

Yeah I was tired of keeping my branches separate, so I merged in my fixes :) If you just want the fixes, (I kept them in separate commits) you can grab them from the commits either via cherry pick or a rebase -i

joshimoo commented 4 years ago

Sec I will rebase them for you, then you can grab them without my pregnancy additions :) There is a separate pull request already for them, and @mipen just merged the smelting fixes, so they don't need to be in here anymore.

wfswanepoel commented 4 years ago

No need, I already fixed it in a different PR which I jsut closed because you also fixed it. I was just a bit earlier to fix it ;)

I am currently using your feature + another feature to modify influence in my own beta branch now which I maintain locally.

joshimoo commented 4 years ago

Great that was the idea, I don't expect these to be merged till 1.3 becomes stable. I mainly create these pull requests so that other devs that want to play 1.3 beta and continue working on their mods can just grab the fixes :)

If you have not already also consider taking the updated modlib fixes from here: https://github.com/mipen/ModLib/pull/24

Vispilio commented 4 years ago

so why is the mod owner not updating his own mod to 1.3,

do people now have to follow Mod beta branches, on top of Bannerlord beta branches ? :))

It's a little crazy even for Covid lockdown standards...

Alevale commented 4 years ago

@Vispilio I assume you are being sarcastic right? If not I assume this hasn't been merged cause it's probably not backwards compatible with 1.2... so it makes no sense to merge this before 1.3 is merged as the default bannerlord version.. otherwise people isntalling the normal mod and being in 1.2 would be complaining the mod broke their game