Wrye-bash: Refactor `cint.py` to remove dependencies on `CBash` instance from external modules

[leandor]

Currently cint.py is missing some very minor support functions that would allow other modules to stop depending on the internal CBash (the ctypes DLL instance) altogether.

Basically, what's missing is some functions to return the version number, and a flag indicating whether the DLL was found and the API is enabled. _(Most common usage is just check whether the instance is not None.)_

The idea is to completely hide the CBash internal DLL instance from external modules.

I'm gonna do this very minor update on a new private branch, and afterwards I'll rebase mi cbash070 branch on top of that one.

(refs.: see conversation in wrye-bash/CBash#1 for more context)

[Utumno]

Nice thanks - looking forward to the changesets. Changed the label to TODO as enhancements are used by the scripts in the meta repo to generate second post and changelogs - so user visible. This is an internal refactoring that's not of much use to users - that's what TODO is more or less (see uses: https://github.com/wrye-bash/wrye-bash/labels/TODO). Some of them turn to enhancements at some point

[leandor]

Right! I hesitated a bit on that one... gotta learn bit by bit. I'll take notice for next time.

[Utumno]

No worries - I had tried myself to encapsulate CBash dll global, but not hard - I'll be real glad to see it go :)

[leandor]

I hope you are not disappointed by the change :)

I added a class (mostly just as scope) and added a few static variables that hold the info that normally other modules use. This way I didn't have to mess with the imports too much, and simplified a bit the building of the version info by having it pre-built.

I'm about to push in a couple of mins... just checking some more stuff and making sure Bash still runs :P

EDIT: wrong word

[leandor]

And.... done!

Oh, forgot to say: I've ran Wrye Bash and built a Bashed Patch in full as a test.

The new branch is called 'leandor-cint-update' and is based on dev.

[Utumno]

You know what - thanks :)

I's very important that the commits are incremental and compile fine - that's an iron rule for commits on dev. I added some wrapping and removed a comment that just shows that CBash global annoyed me too but kept your style - let's not be too nitpicky here!

Incidentally - I was used to work alone here for the past three years and they were three years full of learning both python and the code base. My style conventions have evolved considerably - will update the coding style wiki asap, but it's difficult to convey all this experience in a wiki article anyway. So learn by example :)

If there is one thing that has not changed however is that _negative lines commits_ are preferred. "Less code more features" is a great motto. This leads me to keeping small methods to one line (sometimes) - unless wrapping (which is an important code metric) is violated. Not only for reducing line count but cause I find this less verbose actually - in this I may diverge from standard.

Small points - last, PEP8. New code I write is PEP8 mostly (meaning basically lowercase_with_underscore for vars and attributes). However PEP8'ing Bash is a work for Sisyphus, plus wx violates this (which is a good thing, as this way wx attributes sort in the debugger apart among other things). PEP8 will be revisited later but for now just bear with my small nitpick re: style, here and there.

[leandor]

Thanks! Gotta keep in mind all that for when I start naming modules and exports for the new binding wrapper.

Old exports will probably stay they way Waruddar named them, until I can provide a direct alternative. I intend to respect Python naming conventions for all new indentifiers I introduce.

[Sharlikran]

This code is now in dev and causes an exception.

Wrye Bash starting
Using Wrye Bash Version 307
OS info: Windows-7-6.1.7601-SP1
Python version: 2.7.8
wxPython version: 2.8.12.1 (msw-unicode)
input encoding: None; output encoding: None; locale: ('en_US', 'cp1252')
filesystem encoding: mbcs 
Searching for game to manage:
bush.py   71 _supportedGames: Detected the following supported games via Windows Registry:
bush.py   73 _supportedGames:  oblivion: G:\Games\steamapps\common\Oblivion
bush.py   73 _supportedGames:  skyrim: F:\Games\steamapps\common\Skyrim
bush.py   73 _supportedGames:  fallout4: C:\Games\steamapps\common\Fallout 4
bush.py  131 _detectGames: Detecting games via the -o argument, bash.ini and relative path:
bush.py  137 _detectGames: Set game mode to fallout4 found in parent directory of Mopy:  C:\Games\steamapps\common\Fallout 4
bush.py  172 setGame: No preferred game specified.
bush.py  152 __setGame:  Using fallout4 game: C:\Games\steamapps\common\Fallout 4
fail bash\cint.py
fail bash\bosh\mods_metadata.py
fail bash\bosh.py
Traceback (most recent call last):
  File "Wrye Bash Launcher.pyw", line 88, in <module>
    bash.main()
  File "bash\bash.py", line 365, in main
    import bosh
  File "Wrye Bash Launcher.pyw", line 76, in load_module
    exec compile(code, initfile, 'exec') in mod.__dict__
  File "bash\bosh\__init__.py", line 51, in <module>
    from .mods_metadata import ConfigHelpers
  File "Wrye Bash Launcher.pyw", line 63, in load_module
    mod = imp.load_source(fullname,filename+ext,fp)
  File "bash\bosh\mods_metadata.py", line 32, in <module>
    from ..cint import ObBaseRecord, ObCollection
  File "Wrye Bash Launcher.pyw", line 63, in load_module
    mod = imp.load_source(fullname,filename+ext,fp)
  File "bash\cint.py", line 358, in <module>
    class CBashApi (object):
  File "bash\cint.py", line 361, in CBashApi
    VersionMajor = _CGetVersionMajor()
ImportError: caused by ImportError(u'caused by ImportError(u\'caused by NameError("name \\\'_CGetVersionMajor\\\' is not defined",)\',)',)

[leandor]

Hey Sharl,

Hmmm... I think it's because you're running in an installation without CBash? That's the only way I can see it failing like that. As in, the if CBash: at L184 is not triggering, so the symbols aren't being defined.

That's a bug that my change introduced because I didn't think about what would happen if CBash was not present, sorry.

I pushed a quick fix to a new branch called leandor-tempfix, please test it if you can, at least it should work until @Utumno gives it a look to see if the way I fixed it is OK.

I would perhaps prefer a better fix, maybe adding an else: to the original if, redefining the required symbols to dummy functions.

[leandor]

I've reopened so the issue it's visible until we merge it again to dev.

[Sharlikran]

Okay thanks I'll test it tomorrow.

[Utumno]

How come CBash is not present however ?

[Utumno]

Actually I went ahead and merged cause I needed to push my changes too (gosh what a tricky merge) and the rule for regressions is that they should be fixed as close as possible to the commit that introduced them. Feel free to amend the fix later and no worries :)

[leandor]

The fix was a bit more verbose than I'd want, but I've tested and it worked.
No point in spending too much time refactoring cint.py now since I hope some time in the future it'll be replaced by something smaller and leaner.

I hope it doesn't bring more issues, though! :)

[Sharlikran]

I tried dev just now and it seems to be working fine. Thanks.

[Infernio]

TODO doesn't make sense here (unless there was a larger goal issue that this is a part of that I missed?), so I changed this to enhancement to make it show up on the changelog.

[Utumno]

TODO in the sense of an internal code thing the users won't need to know about IIRC

[Infernio]

Then I should probably change the description for TODO - I took most of those descriptions from the changelog script IIRC. Or maybe we should come up with a label for internal code stuff?

[Infernio]

There, how about that? Will have to go through at some point and tag other fitting issues with it, but the script needs updating anyways :)

[Utumno]

I like refactoring had thought of it myself - yep, and TODO stays for other internal stuff that is not a goal I guess

[Utumno]

And I would say do not bother with closed issues - better concentrate on open (and backburner). It's a dreary place those closed issues. Forward!