Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: 调整 Python Binding 初始化行为 #444

Closed
wants to merge 6 commits into from

Conversation

weinibuliu
Copy link
Contributor

改动详情见 #443

- 调整包的初始化行为:
 - 将检查路径是否存在时改用 pathlib (替换掉原先的 os 库)
  - 当 Libraryopen() 函数返回 None 时,抛出错误
 - 当 `maa/bin` 路径不存在时,抛出错误

- 以下这些文件在初始化时不再检查 Library.initialized 的值:
`buffer.py` `controller.py` `resource.py` `tasker.py` `toolkit.py`

if os.path.exists(__PATH):
# If you want to print maafw version,please use `from maa import ver`
ver = None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个不需要,有 Library.version()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个只是对 ver 进行初始化来着(虽然没什么意义,但这样或许是符合规范的?)
而且使用 ver 来获取版本信息从结果来看也比较直观,因为 open() 的返回值实际上就是 version() 的返回值,所以直接访问 maa.ver 的好处在于无需再运行一遍 version() 函数,并且代码中不需要 import maa.Library (不过需要 from maa import ver),玛丽佬觉得呢

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我感觉没有必要,就是一个简单的获取接口,py 这边没必要缓存一份

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我感觉没有必要,就是一个简单的获取接口,py 这边没必要缓存一份

因为原设计就是缓存了一份 ver ,或许已经有人这么用了?如果不缓存 ver ,完全可以省略掉这个变量

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

去掉吧没必要搞个这个

@MistEO
Copy link
Member

MistEO commented Dec 4, 2024

另外 CI 里的 test 怎么没过要看一下(

@weinibuliu
Copy link
Contributor Author

weinibuliu commented Dec 4, 2024

另外 CI 里的 test 怎么没过要看一下(

https://github.com/weinibuliu/MaaFramework/blob/dev_fix/source/binding/Python/maa/__init__.py
没过是因为 test 里面不存在 maa/bin 目录。test 里的 dll 似乎是放在 insall/dll 里面了。修改之后能过 test ,但真的要这样做吗(

@MistEO
Copy link
Member

MistEO commented Dec 4, 2024

或者就不要 raise FileNotFound ,是不是可以(

@weinibuliu
Copy link
Contributor Author

或者就不要 raise FileNotFound ,是不是可以(

是可以的,本来也是这么设计的,但这样对吗(

Comment on lines 16 to 18
ver = Library.open(__PATH)
if not ver:
raise RuntimeError("Fail to open the library.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

是不是直接可以在 open 里面 raise 了,而不是在这里(

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

是不是直接可以在 open 里面 raise 了,而不是在这里(

也是可以的,那就直接去掉 __init__.py 里面的 ver 变量吗

@@ -13,10 +13,6 @@ class StringBuffer:
_own: bool

def __init__(self, handle: Optional[MaaStringBufferHandle] = None):
if not Library.initialized:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initialized 这个变量看看能不能也删了

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initialized 这个变量看看能不能也删了

或许是可以的,这个变量的用途应该只是在其他文件中检查 library 初始化状态,现在在初始化失败时直接 raise 了

@weinibuliu
Copy link
Contributor Author

weinibuliu@4d4bb25 中进行了一些调整:

  • __init__.py 不再对初始化失败抛出错误,而是在 library.py 中抛出错误
  • 如果命令行传入了 bin 路径,则只使用命令行传入的 bin 路径(目前的设计中,命令行是否还会传入其他参数?因为目前是固定访问索引为1的值)

@MistEO
Copy link
Member

MistEO commented Dec 4, 2024

weinibuliu@4d4bb25 中进行了一些调整:

  • __init__.py 不再对初始化失败抛出错误,而是在 library.py 中抛出错误
  • 如果命令行传入了 bin 路径,则只使用命令行传入的 bin 路径(目前的设计中,命令行是否还会传入其他参数?因为目前是固定访问索引为1的值)
if len(sys.argv) >= 2:
    __PATH = Path(Path(sys.argv[1]).resolve(), "bin")
else:
    __PATH = Path(Path(__file__).parent, "bin")

Library.open(__PATH)

这样就行吧

@weinibuliu
Copy link
Contributor Author

weinibuliu commented Dec 4, 2024

weinibuliu@4d4bb25 中进行了一些调整:

  • __init__.py 不再对初始化失败抛出错误,而是在 library.py 中抛出错误
  • 如果命令行传入了 bin 路径,则只使用命令行传入的 bin 路径(目前的设计中,命令行是否还会传入其他参数?因为目前是固定访问索引为1的值)
if len(sys.argv) >= 2:
    __PATH = Path(Path(sys.argv[1]).resolve(), "bin")
else:
    __PATH = Path(Path(__file__).parent, "bin")

Library.open(__PATH)

这样就行吧

最好还是检查路径是否存在吧。
如果玛丽佬是说ver变量的话,我的考虑是原先的设计就存在ver变量,如果有人已经这么使用,删除它会导致异常。感觉这个更新不应该成为破坏性更新

@MistEO
Copy link
Member

MistEO commented Dec 4, 2024

ver 没人用这玩意的,无所谓(

路径检查在 open 里做就行了,不用在外面做

@weinibuliu weinibuliu closed this Dec 4, 2024
@weinibuliu
Copy link
Contributor Author

先修下bug(

@Windsland52
Copy link
Member

嗯?

MistEO added a commit that referenced this pull request Dec 4, 2024
改动详情见 #443
讨论历史见 #444

---------

Co-authored-by: MistEO <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants