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

Lints #61

Merged
merged 7 commits into from
Nov 22, 2023
Merged

Lints #61

merged 7 commits into from
Nov 22, 2023

Conversation

naomijub
Copy link
Collaborator

@naomijub naomijub commented Nov 21, 2023

  • Added default lint configuration and applied some extra lints
  • Reduced the area that unsafe blocks were touching
  • Renamed some buttons to improve understanding.
  • In game mode, inspector only show if esc is pressed, maybe we can change it to F1
    image

I had to set dtolnay/rust-toolchain to version 1.73, because version 1.74 has a MIR crash on bevy-inspector

Things I plan to do (unordered):

  • Study other editor components
  • Add support for Rapier
  • Add component become a right click event on the inspector window (or a tab)
  • Make a crates folder with xpbd and prefab
  • Improve component naming
  • Currently when we add a new tab we split the dock region, I want them to append to the dock tab
    image
  • Camera view

@naomijub
Copy link
Collaborator Author

@rewin123 if you want to check the CI please refer to the PR on my repo

@rewin123
Copy link
Owner

Thanks for the huge pull request!

I've reviewed and tested the changes. And it works correct. Unsafe megablock has been destroyed, thanks ^^. The button names are really better now. I also noticed that reasonable refactoring was done in many parts of the code.

I just have a few questions. When building on rust 1.72 I get a warning: "warning: unused manifest key lints (may be supported in a future version)". It seems that lints is only available in the nightly version of rust?

LTO seems to have to be placed in the head crate of the game, as it enables optimization on all underlying crates. Maybe it should be removed from space_editor, since ideologically the editor should be attached as a subproject to the game. (Also lto destroys compile time).

Support for reaper would be great! As well as adding components by right-click (I wanted to do it originally, but I don't see how to implement it in egui).

For third-party dependencies defined via feature I've allocated an "optional" folder, and also allocated a bevy plugin "OptinalPugin" for centralized connection of "optional" third-party modules to the editor. So in "my ideal world", reaper support should be in the "optional/reaper_plugin" folder.

@naomijub
Copy link
Collaborator Author

naomijub commented Nov 22, 2023

Happy to help, I was doing my own editor when I found yours and the way you solved prefabs was way cleverer haha.

I just have a few questions. When building on rust 1.72 I get a warning: "warning: unused manifest key lints (may be supported in a future version)". It seems that lints is only available in the nightly version of rust?

It is supported in 1.74, which should have a fix for the MIR error soon, then I can update the toolchain.

Support for reaper would be great! As well as adding components by right-click (I wanted to do it originally, but I don't see how to implement it in egui).

Will do in a next PR, our editors have a very different organization, so I get a bit lost on migrating stuff. Also, I reimplemented most of the bevy libraries because I didn't want to have dependencies on them upgrading to newer bevy versions, so some of it doesnt really sync with the original work.

For third-party dependencies defined via feature I've allocated an "optional" folder, and also allocated a bevy plugin "OptinalPugin" for centralized connection of "optional" third-party modules to the editor. So in "my ideal world", reaper support should be in the "optional/reaper_plugin" folder.

That makes sense. In my ideal "rusty" world, we can export most of your plugin into a crates folder, especially prefab part, so it could become an external library. Having said that, what do you think of a structure like:
src/ <- has editor executable
crates/

  • prefab <- external crate that can be published to crates.io
  • editor <- everything related to editor
  • shared <- things shared between prefab and editor

modules/ <- the community modules to be featured in the editor (it could be inside crates, but I think it could cause confusion to live with crates.

  • xpbd
  • rapier
  • vx_bevy
  • openxr
  • etc etc

lto was me trying to fix an internal issue, we can probably configure profiles to deal with those preferences. it has some advantages in dev mode

@naomijub
Copy link
Collaborator Author

@rewin123 what is your OS? Found some issues with windows

@naomijub naomijub marked this pull request as draft November 22, 2023 17:44
@naomijub naomijub marked this pull request as ready for review November 22, 2023 18:01
@rewin123
Copy link
Owner

Hmm. Ok, I don't see the problem anymore, then I'll merge the pull request.

I like the proposed file structure, except that shared is a unnecessary folder. I will start to move folder struct too.

My operating system is windows, but i did not found bugs.

@rewin123 rewin123 merged commit 74f1f27 into rewin123:main Nov 22, 2023
@naomijub
Copy link
Collaborator Author

Lets move the discussion on the file structure to an issue?

@rewin123
Copy link
Owner

Yep. I will open

@naomijub naomijub deleted the lints branch November 23, 2023 04:01
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.

2 participants