pigskin / kodi-gamepass

NFL Game Pass add-on for Kodi
Other
123 stars 83 forks source link

[Question] Pigskin Init before __main__ #367

Closed OnceUponALoop closed 2 years ago

OnceUponALoop commented 6 years ago

Was looking into enabling the busy dialog on startup - wondering why this code isn't in main and is instead called on script startup - figured there might be a reason that i'm overlooking

username = addon.getSetting('email')
password = addon.getSetting('password')

proxy_config = None
if addon.getSetting('proxy_enabled') == 'true':
    proxy_config = {
        'scheme': addon.getSetting('proxy_scheme'),
        'host': addon.getSetting('proxy_host'),
        'port': addon.getSetting('proxy_port'),
        'auth': {
            'username': addon.getSetting('proxy_username'),
            'password': addon.getSetting('proxy_password'),
        },
    }
    if addon.getSetting('proxy_auth') == 'false':
        proxy_config['auth'] = None

gp = pigskin(proxy_config, debug=True)

Couldn't this be moved to main so we can display a busy dialog while loading?

if __name__ == '__main__':
    show_busy_dialog()

    username = addon.getSetting('email')
    password = addon.getSetting('password')

    proxy_config = None
    if addon.getSetting('proxy_enabled') == 'true':
        proxy_config = {
            'scheme': addon.getSetting('proxy_scheme'),
            'host': addon.getSetting('proxy_host'),
            'port': addon.getSetting('proxy_port'),
            'auth': {
                'username': addon.getSetting('proxy_username'),
                'password': addon.getSetting('proxy_password'),
            },
        }
        if addon.getSetting('proxy_auth') == 'false':
            proxy_config['auth'] = None

    gp = pigskin(proxy_config, debug=True)
    addon_log('script starting')
    hide_busy_dialog()

If there's no specific reason then i'll create a pull request with the changes 👍

pyrocumulus commented 6 years ago

I changed the implementation recently (https://github.com/aqw/xbmc-gamepass/commit/e646c6f4d94fe180bdb1a490fa19410c001f8b6a) to the new busy dialog. However I only replaced the calls to the old one with calls to the new one.

The call to closing the busy dialog on script startup (instead of starting it) was already there and I kept it there, just replaced it. Why it is there, I do not know.

Perhaps @aqw knows if there is a specific reason?

aqw commented 6 years ago

@OnceUponALoop Sorry for the long overdue response.

There's no specific reason that I can remember, and I agree that it makes more sense in __main__. The code has grown organically over the years, so there's plenty of warts, and I'm a sysadmin who can code, rather than a proper programmer. :-) So any effort towards cleanup and repayment of technical debt is much, much appreciated.

---Alex

jm-duke commented 2 years ago

Doing some clean-up and this issue report is really old, so I'm closing it.