search

nutti / Screencast-Keys

Blender Add-on: Screencast Keys
GNU General Public License v3.0
1.27k stars 112 forks source link

Add new loading features #62

Closed CheeryLee closed 3 years ago

CheeryLee commented 3 years ago

Purpose of the pull request
Feature request

Description about the pull request
Following in the footsteps of https://github.com/nutti/Screencast-Keys/issues/25 . I did some changes to make screencast working in two cases: 1) automatically load at startup if the appropriate setting is on; 2) don't turn off screencast if new scene was loaded in the curent session

nutti commented 3 years ago

@CheeryLee

Thanks for the PR and challenging the difficult feature. I tried your PR, and I found the below issue.

CheeryLee commented 3 years ago

@nutti whoops, my bad. Made a small fix.

nutti commented 3 years ago

@CheeryLee

There is a still error.

2.93\scripts\addons_contrib\screencast_keys\__init__.py", line 119, in get_func
    return ops.SK_OT_ScreencastKeys.is_running()
AttributeError: type object 'SK_OT_ScreencastKeys' has no attribute 'is_running'

Also, is_running should be in SK_OT_ScreencastKeys and global variable is bad way for Blender add-on.

CheeryLee commented 3 years ago

Oh, my bad again, there should be ops.is_running() or ops.running instead of ops.SK_OT_ScreencastKeys.is_running(). Currently this is the one way I see to determine is screencast working or no. I don't think Blender can give any guarantee that operator instance won't be reloaded in a new scene.

But you're right, maybe it's good to know should it be inside operator body.

nutti commented 3 years ago

Thanks. Now, I confirmed it works correctly.

I will check your code in detail from now.

CheeryLee commented 3 years ago

Made changes from the list above

nutti commented 3 years ago

@CheeryLee

Thanks for your work. I think you missed this comment. Could you apply this suggestion? After applying this suggestion, I can merge your patch :)