FraserLee / bevy_sprite3d

Use sprites in a 3d bevy scene.
MIT License
135 stars 22 forks source link

Format code and interest in accepting changes re sprite flipping and sprite anchor #24

Open mgi388 opened 1 month ago

mgi388 commented 1 month ago

Hey @FraserLee are you still maintaining this crate? And if yes, are you open to issues/changes? I'm using your crate and looking into if I can make some enhancements to it including:

If you are open, I think it would be worth doing a quick pass over the codebase and format it like in https://github.com/FraserLee/bevy_sprite3d/pull/19 (but that PR suggests they've made code changes).

As a start, if I submit a PR to format the code without changing any logic, would you merge it?

FraserLee commented 1 month ago

Absolutely! Those enhancements sound great, I'm very open to them.

Quick note on formatting: default cargo fmt is pretty ruinous to the code here (you can check out the discussion on prior PRs for details). I wouldn't be surprised if there's some configuration of formatting options that are a lot better than the defaults, if you'd want to go searching for them please feel welcome. If not, please keep formatting changes manual and reasonable.

Best of luck, I hope to merge your changes soon!

mgi388 commented 1 month ago

@FraserLee awesome, wish me luck! Re formatting, I was hoping to just copy whatever bevy repo uses? I don't care personally about what the format is, I just noticed that in my VS Code when saving it reformats the whole world by default and I want to set the repo up so it avoids that (a la bevy repo).

mgi388 commented 1 month ago

I see your comments on https://github.com/FraserLee/bevy_sprite3d/pull/11. I'll see what rustfmt.toml looks like once applied.

FraserLee commented 1 month ago

On second consideration, I'll keep this issue open for future discussion until it's no longer needed. Feel free to keep posting here 🙂

One thing:

I just noticed that in my VS Code when saving it reformats the whole world by default

I don't want to be a prescriptivist about tools. We all walk our individual paths to a setup that works best for us and us alone, a beautifully unique entangled mess of our own idiosyncrasies, our environment, broader engineering culture, the rest of the causally connected universe in our past light-code. "Correct" is meaningless beyond what feels correct for you. There are no universal truths.

That said.

Editing text and diffing text have been solved problems since the 80s. If your editor can't edit text without destroying any meaning from the diff, you may have a problem with your editor.

I don't use VSCode, but it might be worth checking out something like this: https://tosbourn.com/configure-vs-code-to-only-format-changed-lines/

mgi388 commented 4 weeks ago

I see your comments on #11. I'll see what rustfmt.toml looks like once applied.

Yep it's pretty much as per https://github.com/FraserLee/bevy_sprite3d/pull/11/files and I don't know rustfmt enough to be able to add rules in there off the top of my head to try and match the existing style.

I can add #[rustfmt::skip] in a couple of places to avoid changing some of the arrays, but the arrays are not that large, it doesn't feel worth it.

Anyway, if it were me, I'd just add the bevy rustfmt.toml, apply it (which feels universal in Rust but who knows maybe I know nothing), merge it and move on but it sounds like you'd prefer to leave it as is. All good, it's your decision 🙏

If I can work out a nice DX for making changes, I'll be back in the future with some PRs but I haven't got any of the features mentioned above working yet anyway, so nothing to share yet.

zArubaru commented 3 weeks ago

Hey, just throwing in my 5 cents out of the blue!

Probably best to let rustfmt handle most of the formatting (as contributors will usually auto format, afaik), then add #[rustfmt::skip] for the tricky parts (and edit rustfmt.toml for any other specifics you need). Otherwise trying to stick to a single style is going to be a pain :)