bevy_sprite3d icon indicating copy to clipboard operation
bevy_sprite3d copied to clipboard

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

Open mgi388 opened this issue 1 year ago • 6 comments

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:

  • sprite x/y flipping
  • supporting changing sprite anchor (useful when changing texture atlas index and the new sprite has a different anchor/pivot)
  • I wondered about your choice of pivot coordinate space because it differs to Bevy's (0.0, 1.0) vs (-0.5,0.5)

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?

mgi388 avatar Aug 14 '24 11:08 mgi388

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!

FraserLee avatar Aug 15 '24 06:08 FraserLee

@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 avatar Aug 15 '24 07:08 mgi388

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

mgi388 avatar Aug 15 '24 07:08 mgi388

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/

FraserLee avatar Aug 15 '24 07:08 FraserLee

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.

mgi388 avatar Aug 15 '24 08:08 mgi388

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 :)

zArubaru avatar Aug 23 '24 14:08 zArubaru

This isn't planned, so closing.

mgi388 avatar Dec 02 '24 04:12 mgi388