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

🚧 Split luminol into separate crates #45

Merged
merged 58 commits into from
Nov 22, 2023

Conversation

melody-rs
Copy link
Member

This is currently WIP. The goal of this PR is to split Luminol into separate crates and ditch interior mutability, hopefully solving a whole class of bugs related to opening and closing projects.

white-axe added a commit to white-axe/Luminol that referenced this pull request Oct 12, 2023
The Rust documentation says you need to enable the write flag to use
create or truncate when writing to files, so using the create flag
without also using the write flag is probably undefined behaviour.

It seems there are still more problems preventing saving from working,
but I assume this is in the scope of Astrabit-ST#45 so I will
make no further changes.
melody-rs pushed a commit that referenced this pull request Oct 12, 2023
The Rust documentation says you need to enable the write flag to use
create or truncate when writing to files, so using the create flag
without also using the write flag is probably undefined behaviour.

It seems there are still more problems preventing saving from working,
but I assume this is in the scope of #45 so I will
make no further changes.
crates/luminol-audio/src/wrapper.rs Outdated Show resolved Hide resolved
@melody-rs melody-rs requested a review from white-axe November 20, 2023 21:13
crates/luminol-ui/src/tabs/started.rs Outdated Show resolved Hide resolved
@white-axe
Copy link
Collaborator

Does the map editor work in web builds yet? When I try opening the map editor I start getting WebGPU errors and the screen turns blank.

@melody-rs
Copy link
Member Author

It did before, I'm not sure why that's happening? I have a sneaking suspicion it has something to do with use_push_constants not affecting shaders properly

I was going to pull in naga_oil to replace the const_format stuff in a separate PR but I'll try it now and see what happens

@melody-rs
Copy link
Member Author

@white-axe Should be fixed now!

@melody-rs melody-rs requested a review from white-axe November 21, 2023 07:45
Copy link
Collaborator

@somedevfox somedevfox left a comment

Choose a reason for hiding this comment

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

In my opinion, luminol- prefix should be omitted in the crates directory. The fact that they belong to Luminol is self-explanatory because of the root package manifest and git repository these crates are in.
i.e.

crates
└ ━> app
└ ━> audio
└ ━> components
└ ━> [...]

@melody-rs
Copy link
Member Author

Ok, I'll do that! I'll still keep the prefix in the crate names though to avoid collision with module names and because I intend to upload luminol to crates.io in the future.

@melody-rs melody-rs requested a review from somedevfox November 22, 2023 06:13
@melody-rs melody-rs merged commit 6998d64 into Astrabit-ST:dev Nov 22, 2023
@melody-rs melody-rs deleted the split-crates branch November 22, 2023 07:49
@melody-rs melody-rs mentioned this pull request Jul 11, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rework modal system Move error type Result<T, String> to a custom type Fault tolerance
3 participants