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

Main module improvements, build improvements and other bits #1302

Draft
wants to merge 42 commits into
base: master
Choose a base branch
from

Conversation

C0rn3j
Copy link
Contributor

@C0rn3j C0rn3j commented Nov 18, 2024

Supersedes #1285

Apologies for the size of this PR, which will increase still, it could in theory be split into smaller parts if really needed.

  • Fix spamming 2021 on launch from t_extra as a random bit of example code was hanging there
  • Tear out all remaining start imports
  • Put optdepends into requirements_optional.txt
  • Put most of the codebase under logging module and log all exceptions
  • Fix trying to load Windows version on non-Windows systems
  • Remove seemingly useless macOS code for tap.stop(), tap is not defined anywhere in the codebase
  • Fix transfer-playtime-to not copying tags
  • Fix run.sh not deleting egg build folder
  • Make pyproject.toml packaging package assets, themes, templates, modules.
    • Parts of the projects were consolidated under the standard directories - src/tauon
    • Modules are now imported as from tauon.t_modules.t_extra - I suggest dropping the t_ prefix in the future, now that this is packaged together.
  • Remove duplicated alpha_blend function
  • Split migration code from t_main into t_db_migrate
  • Removed a piece of code that was always trying to run with an undefined enc variable
  • Fix broken pylast import
  • Upgrade Gtk to 4.0 in t_main, this complements an earlier fix, 3.0 should no longer be accidentally imported and clash with 4.0
  • Converted parts of code from os file calls to Path from pathlib
  • Converted first 1/10th of t_main from spaces to tabs
  • Fixed reload_albums() in t_main using unbound current_date and current_title
  • BS4 was used in t_main without being imported

TODO:

  • Fix Found XDG-Music (uh no you didn't) #1240, without XDG implemented this code will crash due to the improved packaging clashing with hardcoded paths
  • Put in translations and anything else that's necessary into pyproject.toml too
    • Advice how to resolve that one welcome.

@C0rn3j C0rn3j marked this pull request as draft November 18, 2024 23:01
@Taiko2k
Copy link
Owner

Taiko2k commented Nov 18, 2024

An asset not being present may be warning level but not critical. The app should try to run no matter what state its in unless user data integrity is at risk.

@C0rn3j
Copy link
Contributor Author

C0rn3j commented Nov 18, 2024

If that asset is not present, it crashes below on SDL trying to use it, but with a much less clear and uncaught error, so I verify that the asset is even there before using it in the first place.

It'll crash on raw_image.contents being a NULL pointer, to be specific.

@Taiko2k
Copy link
Owner

Taiko2k commented Nov 18, 2024

Oh that's fine then.

@C0rn3j
Copy link
Contributor Author

C0rn3j commented Nov 18, 2024

I've edited in a note about reload_albums() as you'll probably know what to do there much quicker than me

@Taiko2k
Copy link
Owner

Taiko2k commented Nov 18, 2024

I've edited in a note about reload_albums() as you'll probably know what to do there much quicker than me

I can't seem to find that, can you link to or paste it here?

@C0rn3j
Copy link
Contributor Author

C0rn3j commented Nov 18, 2024

https://github.com/Taiko2k/Tauon/blob/c80561e0dd56f036d99251040c0b72c1dc23f046/t_modules/t_main.py#L27222-L27307

EDIT: It's the last TODOs note in this PR description, I didn't realize you mean the entire note at first

@Taiko2k
Copy link
Owner

Taiko2k commented Nov 19, 2024

Oh, current_date and current_title can/should be defined near the top of the function, next to current_folder = "" etc. Currently it logically just werks but that should make it more readable.

@C0rn3j
Copy link
Contributor Author

C0rn3j commented Nov 19, 2024

Currently it logically just werks

I am not sure about that, before the commit with your suggestion that I just sent, the value will imo always be None for the comparison.

@C0rn3j
Copy link
Contributor Author

C0rn3j commented Nov 19, 2024

Another thing - when the module is unable to be imported, a message is thrown to inform the user with what's missing due to the failed import:

	logging.warning("Unable to import pypresence, Discord Rich Presence will be disabled.")

But it's not obvious to me what's missing with opencc(something with subtitles I guess) and natsort(Faster sort?):

try:
	import opencc
except ModuleNotFoundError:
	logging.warning("Unable to import opencc, TODO(Martin): <what is lost?>.")

try:
	import natsort
except ModuleNotFoundError:
	logging.warning("Unable to import natsort, TODO(Martin): <what is lost?>.")

@Taiko2k
Copy link
Owner

Taiko2k commented Nov 19, 2024

If a value is undefined in Python it will crash, not be None. Unless those happen to be shadowing a global variable but would then crash on attempt to assign them. But here because its an if/else statement, and i == 0 will always be true the first time those variables never get evaluated before being assigned, so it works.

Theres also current_artist btw.

@Taiko2k
Copy link
Owner

Taiko2k commented Nov 19, 2024

  • opencc: The ability to perform search using Traditional Chinese and Simplified Chinese interchangeably.
  • natsort: Used for sorting algorithms. The sort order of a playlist may not be the optimal intention if this is missing.

@Taiko2k
Copy link
Owner

Taiko2k commented Nov 19, 2024

Upgrade Gtk to 4.0 in t_main, this complements an earlier fix, 3.0 should no longer be accidentally imported and clash with 4.0

This issue prevents the app from starting for me so Ive committed this fix ahead of this PR.

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.

Found XDG-Music (uh no you didn't)
2 participants