wrye-bash / CBash

Home of CBash.DLL sources
GNU General Public License v2.0
9 stars 4 forks source link

CBash (0.7.0) breaks compat with pre-existing cint.py API #1

Closed leandor closed 4 years ago

leandor commented 7 years ago

It's nothing major, I've already patched the needed changes on my local copy.

Basically this is what has changed on CBash's side:

I'm gonna make a Pull Request on Wrye Bash for these changes when I'm sure that the new CBash version works and has no 'hidden' problems.

leandor commented 7 years ago

@Utumno:

Should I rollback the renames, and provide a default for the new argument added to keep current WB compatible? Or should I move forward with those changes as given?

This is considering that it can be some time before the new binding module is usable, so maybe having a fully compatible CBash is something to consider until then.

Utumno commented 7 years ago

Hmmm can't readily tell - do you want to keep the renames ? The progress callback is a nice idea I think.

leandor commented 7 years ago

No problem either way for me! It would be easier to keep the renames :) They are innocent enough to be sure that that shouldn't cause any issues on your side, I hope!

As for the callback, I agree... I intend to keep it, just needs to have a safety check to insure everything works OK when the calling method doesn't provide one.

Utumno commented 7 years ago

The only proble I see it's we can't swap 0.6 for 0.7 - users may need to do that for testing ? Or we just move forward and don't look back ? Probably the latter

leandor commented 7 years ago

Yeah, that's the bit that is a problem. The need to keep a couple of files in sync, basically.

If you swap CBash you also need to swap cint.py, and two other py files too (Mopy/bash/basher/mod_links.py and Mopy/bash/basher/__init__.py)

Maybe, if you want, I could do a pretty small refactoring on another branch. and break my original commit[1] in two steps:

  1. I could first refactor cint.py and the other two files by changing them to depend just on a cint.py provided API, adding an export that gives the info they need, instead of using CBash functions (all that while still using CBash 0.6.0), and then, afterward,
  2. rebase my wip branch[1] on top of that other branch's head.

[1] reference: wrye-bash/wrye-bash@67c2da1400133df56d2098c49be6598d648e785c

That way, all the rest of Bash's code would depend on cint.py API and you'd just need to keep cint and CBash in sync.

EDIT: Rebased my wip branch to current dev

Utumno commented 7 years ago

As I see the other two files have just duplicated code - time for a nice function in cint yes ;)

Utumno commented 7 years ago

Hi there @leandor! Hope all is well :)

I am ready to roll out a second beta for 307 - I would like to include CBash 7.0 (and cleanup the issues here). While it may have rough edges (or exactly because of that) the best way to test it is to make it available to the users - we can always fix it/rollback for 307 proper - and it must be in 307, I just can't have another RecordProcessor: Information lost nonsense coming up in the forums.

I would like you however to go through the readme and update CBash info in there (including linking to this repo etc). Let me know if possible.

Cheers!

Infernio commented 4 years ago

CBash 0.7.0 was merged into mainline Wrye Bash in wrye-bash/wrye-bash@48356a3244425e9badf253842790a7833eab10c8, follow-up to remove a now-pointless cint.py try-catch check in wrye-bash/wrye-bash@9e222e0ff0fda3f0c0d5b106d7f9b028a1f7791a.