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

Feat/multiassetpath #580

Open
wants to merge 12 commits into
base: alpha-dev
Choose a base branch
from

Conversation

KRiedmiller
Copy link

Parse asset paths containing wildcards. This is useful for loading multiple .obj files at once into multiple SceneParts.

Implements a new MultiAssetPath type. Only functions which want to use this feature need to switch from the unchanged AssetPath to the new MultiAssetPath.

This PR is based on the PR #572 branch

Fixes #557

Copy link
Collaborator

@dokempf dokempf left a comment

Choose a reason for hiding this comment

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

Looks great - I am only unsure about patterns based on absolute paths, see my comments.

@KRiedmiller
Copy link
Author

Thanks for the review, I addressed your comments and fixed the absolute path logic.

@dokempf dokempf self-requested a review March 4, 2025 16:26
Copy link
Collaborator

@dokempf dokempf left a comment

Choose a reason for hiding this comment

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

Great thanks! Will merge after resolution of #572

@dokempf
Copy link
Collaborator

dokempf commented Mar 7, 2025

@KRiedmiller I just realized, that we need this not only for ScenePart.from_obj, but also for ScenePart.from_xml.

)
)

return [cls._from_cpp(part) for part in _cpp_parts]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Couldn't we just implement this via [ScenePart.from_obj(f) for f in obj_files]. THat would not duplicate the underlying logic.

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.

Add MultiAssetPath validation logic to support extended filepath feature
2 participants