#0.14 Bevy Jam

1 messages · Page 8 of 1

slender belfry
#

That's pretty neat 😄

#

Yeah that's definitely something that would be neat as an own crate, but may be confusing / increasing the mental burden in a template imo

bitter sparrow
slender belfry
#

thanks!

#

Then the only things left from Alice's notes are the asset stuff (PR is ready for review there), the spawning stuff you're working on and the game module

bitter sparrow
#

imo game -> demo and call it good

slender belfry
#

not quite imo

#

The asset module is not something you will yeet

#

Neither the audio stuff

bitter sparrow
#

well, we can pull some stuff out of there

#

animation, movement, and player (my PR removes spawn/ and level) can remain in demo

#

also i'd like to rename ui -> theme at some point

slender belfry
#

Was the current vibe that we had too much or too little nesting and grouping?

#

I forgot

#

I think it changed at some point?

bitter sparrow
#

i think too much has generally been the vibe

#

for modules

#

well my pr compiles but he's tiny 😄

2024-08-05_1722902070_379x300.png
slender belfry
bitter sparrow
#

i've kinda been coming around to less nesting than before (especially wrt game), but

#

i am still personally strongly in favor of a util/ and a core/

slender belfry
#

Same

bitter sparrow
#

but i understand that bevy_quickstart is a pretty small template so as it currently stands, the template itself doesn't need those folders

slender belfry
#

But I won't die on that hill if cart himself is strongly against it 😄

slender belfry
slender belfry
bitter sparrow
#

i also noticed in some bevy jam games, some people really like having almost everything in the top-level folder

#

ah the level entity is missing a SpatialBundle, and i'm now spawning the player as a child

#

i'm surprised the player renders at all like that

#

well ig it makes sense

#

oh i forgot to move the buttons into separate systems so they can register their one-shots in-place

inland wyvern
slender belfry
slender belfry
slender belfry
bitter sparrow
#

and they aren't actually spawning the player, they're inserting the necessary components onto the already-spawned empty player entity

#

so you can also do commands.spawn(my_bundle).add(my_thing) etc

slender belfry
bitter sparrow
#

current naming makes perfect sense to me personally

slender belfry
slender belfry
bitter sparrow
#

for example, we don't call NodeBundle something like InsertNodeBundle, but we're not confused about commands.spawn(NodeBundle)

#

even though internally what it's doing is commands.spawn_empty().insert(NodeBundle)

slender belfry
bitter sparrow
#

fair 😄

#

"create a player entity" may be more realistic

#

or "construct" 👀

#

i have not read the bsn proposal yet

slender belfry
bitter sparrow
#

plugin fns are named like nouns too

slender belfry
#

Sure, hence "generally"

#

I'll admit I cannot come up with a better name

slender belfry
bitter sparrow
#

well, the function player is the player spawning system

#

:p

#

i mean

#

you can think of the system as representing the entity itself

#

which makes more sense with e.g. struct Projectile

slender belfry
#

I'd be on board if it was in a spawn module

#

spawn::player makes sense

bitter sparrow
#

but not really

#

commands.spawn_fn(spawn::player)?

#

and commands.add_fn(spawn::player) as well

slender belfry
bitter sparrow
#

the function represents an entity, what you do with that entity is the commands.xyz call

slender belfry
#

I'm not arguing against the calling site, that's completely fine

#

I'm saying that the definition is a potentially confusing thing

slender belfry
#

If anything, it represents, a kind of mapping for that entity

#

not quite a map, but some kind of processing pipeline for the entity

#

That pipeline analogy would be stronger if we returned the Entity at the end. That way, you could progressively add stuff to it.

#

Not sure if that makes sense though

bitter sparrow
#

we do return it from the commands.spawn_fn though

#

just not from the system itself

slender belfry
#

Yeah, true

#

process_player?

bitter sparrow
#

ok how about this

#

the systems are analogous to plugins, but instead of operating on App, they operate on an entity

#

in which case the naming convention is consistent

slender belfry
bitter sparrow
#

ig we do do that

#

at least wrt it being a noun it's consistent

slender belfry
#

spawn?

#

I know it's a bit of a lie

#

but it's intuitive

bitter sparrow
#

like player_spawn?

slender belfry
#

Just fn spawn

bitter sparrow
#

oh but we have different things we want to spawn tho

#

potentially in the same file

slender belfry
#

Fair enough

#

I'll have to think about this for a while. Can't come up with anything nice right now.

slender belfry
bitter sparrow
#

yeah for sure

agile dirge
#

pattern appr, but i went over it quickly, i can review better tmrw

bitter sparrow
#

sounds good, i'll leave it up for a while probably. i need to update the design doc as well

slender belfry
#

I like your doc comment calling it construct

#

What if we called it fn construct and, if we need multiple per file, we added a mod around the function?

#

I'm also fine with calling them construct_*

slender belfry
#

About add_fn, wouldn't it be more consistent with rust to call it insert_with?

bitter sparrow
#

so the parallel is to add, which is for adding an EntityCommand, which is what's being used for the data-having version

#

i'll admit that the spawn -> insert vs. spawn -> add parallel is a little odd

#

i'll think whether the whole EntityCommand thing can be skipped entirely now

#

well it could but we'd need a new trait to replace it..

#

basically if you want to do commands.spawn_with(MyStruct), we need to be able to associate MyStruct with a particular system

#

currently that link is created by writing the 5-liner impl EntityCommand for MyStruct { world.run_system_once_with((id, self), Self::spawn); }

#

ideally we would have a trait with Self::spawn as a trait method. but then it wouldn't be able to accept any system params while still matching the trait's definition

#

so since MyStruct is technically an EntityCommand, it can be applied via EntityCommands::add for free, which is why add_fn comes in to match

slender belfry
#

I left some review comments

bitter sparrow
#

yeah it's analogous to struct vs function form of a command

#

where the struct can contain data, like struct Projectile { lifetime, .. }

slender belfry
#

Hope my review comments don't sound too negative. I like the pattern and think something like this should be in Bevy proper, I'm just not happy about the onboarding.

bitter sparrow
#

so what's happening is that in present-day bevy, "spawn" only supports bundles. so spawn = bundle, and insert = bundle

#

and commands are fully relegated to "add"

#

IMO if I could change bevy proper however i want rn

#

i'd make spawn take an EntityCommand, impl EntityCommand for Bundles, and remove insert

#

assuming rust doesn't complain about the blanket impl

#

so you would .add a bundle to insert it

#

then spawn_with is not needed because it's just spawn, and there's no insert vs add weirdness

#

and i'd implement EntityCommand for systems taking In<Entity> as their first arg, so spawn_fn and add_fn wouldn't be needed either

#

again assuming that's even possible

slender belfry
bitter sparrow
#

so you'd have spawn(bundle), spawn(entity_command), spawn(entity_fn), and add(bundle), add(entity_command), add(entity_fn)

slender belfry
#

RFC time?

bitter sparrow
#

well i'm not sure rust will even allow it 😄 but i can test it out later

slender belfry
bitter sparrow
#

icic

#

i think that requires owning EntityCommand because of orphan rule at least, so it would have to be upstream

slender belfry
slender belfry
slender belfry
#

I could turn the involved constants into functions returning String, but that feels a bit weird.

#

I could also wrap some HashMap functionality behind functions that take impl Into<String>, but that would make the thing more complex for little gain, so I don't want to do that.

prisma delta
#

But IMO you should just simplify this further to take a 'static str

slender belfry
slender belfry
#

Since the underlying HashMap is storing String, we cannot pass &str to it.

#

Except by wrapping everything with custom functions and constructors that accept some variant of impl Into<String>, impl Into<Cow<'_, str>>, impl AsRef<str>, or impl Borrow<str>, but that again is just more boilerplate

slender belfry
slender belfry
#

@bitter sparrow @bright ledge I just realized something. If we register a one-shot system when we create a button, is that not a memory leak? AFAIK we cannot "deregister" a system later, but we can very well build the same button multiple times when re-creating a menu. Wouldn't the storage holding the one-shot systems just grow and grow?

#

It's not that critical since it's very little memory, and it increases only rarely. But still, it makes me a bit nervous.

bright ledge
#

I'm not sure

#

I'm fairly sure since we're storing them in a component once the buttons despawned it should delete the SystemId

#

so it should be fine

bitter sparrow
slender belfry
#

they are not handles

bright ledge
#

wouldn't despawning the button get rid of it though

#

since it's attached

#

oh wait no

slender belfry
bitter sparrow
#

it'd get rid of the SystemId but not the one-shot system entity, since we don't have relations or anything to do that kind of cleanup

bright ledge
#

I see what you mean

slender belfry
#

Now you just deleted the only way to access that memory

bitter sparrow
#

well you can still access with a query :p

#

but you'd be accessing all the other one-shots as well

slender belfry
#

Oh, one-shot systems are components?

bitter sparrow
#

i mean they're entities one way or another

#

idk what components those entities have

bright ledge
bitter sparrow
# 
    pub fn register_boxed_system<I: 'static, O: 'static>(
        &mut self,
        system: BoxedSystem<I, O>,
    ) -> SystemId<I, O> {
        SystemId {
            entity: self
                .spawn(RegisteredSystem {
                    initialized: false,
                    system,
                })
                .id(),
            marker: std::marker::PhantomData,
        }
    }
slender belfry
bitter sparrow
#

yea

#

i copy/pasted the World version of this :)

slender belfry
#

So we could clean them up?

bright ledge
#

I think so

slender belfry
#

Hmm

#

I don't like this

bitter sparrow
#

i mean

#

we can attach our own marker component lol

#

SystemId::entity() and then attach

slender belfry
bitter sparrow
#

or

#

what if we made the systemid a child of the button?

slender belfry
#

Now if we had relationships, the rest would be trivial

bitter sparrow
#

so despawn_recursive cleans it up automatically

slender belfry
#

I think that should work

bright ledge
#

but wouldn't that run into the problem of system id storting the entity

bitter sparrow
#

question is if that's too ugly for the template compared to using observers instead of one-shots

slender belfry
#

Not sure if at that point we should just switch to observers though

slender belfry
bright ledge
#

yeah

slender belfry
bitter sparrow
#

one-shots being potential memory leaks is an interesting footgun though. i haven't heard about it before

bright ledge
#

if the system id is a child and then we despawn it we run into the same problem of the SystemId being an entity and it wouldn't actually delete the stored system

bitter sparrow
#

oh i meant the actual system entity as a child

bright ledge
#

ohh

#

are we able to do that?

bitter sparrow
#

yeah, SystemId internally includes its Entity id

bright ledge
#

oh yeah

bitter sparrow
#

which you can access with a method

bright ledge
#

just saw your code

#

yeah that's interesting

slender belfry
#

Ignoring the whackiness, that would be a pretty cool interaction, hehe

bright ledge
#

yeah that could definitely work

bitter sparrow
#

i'm personally fine with either approach

#

the closure / one-shot approach is a nicer API

bright ledge
#

it's also easily extendable which is the main benefit personally

slender belfry
#

Update: adding a closure directly to fn button is rather cumbersome, as ChildBuilder cannot register one shot systems directly, which means we would have to re-implement it:

let entity = self.spawn_empty().id();
self.push(RegisterSystem::new(system, entity));
SystemId::from_entity(entity)
#

which is very very meh

bitter sparrow
#

it should be able to

bright ledge
#

couldn't you use impl IntoSystem as the button type

bitter sparrow
#

you can do ChildBuilder::add_command

bright ledge
#

which I'm fairly sure it what register system uses

slender belfry
slender belfry
bitter sparrow
#

yeah another bevy gap

#

i have extension traits for all of these gaps so i forget anyways

slender belfry
#

You need to do it manually

bright ledge
#

ahh

#

got it

slender belfry
bitter sparrow
#

would be cool if EntityCommands had register_system :)

#

maybe a little confusing though idk

bright ledge
#

isn't EntityCommans meant to be scoped to the entity being referenced

#

which might be why they're not doing it

bitter sparrow
#

yeah, i was thinking like it would spawn the one-shot system entity as a child

prisma delta
#

Can't you get Commands back out of EntityCommands?

bitter sparrow
#

you can, but not really out of ChildBuilder

#

it only offers .add_command

#

oh

prisma delta
#

😔

#

ChildBuilder makes me so frustrated

bitter sparrow
#

right you can child_builder.spawn() and then get the .commands() out of that EntityCommands lmao

#

and then despawn the entity because you only spawned it to access Commands, obviously /j

slender belfry
bitter sparrow
#

yep i get that on your pr as well

slender belfry
#

Oh actually I cannot open any PRs on bevy_quickstart anymore

#

Also Bevy's PRs

#

F in the chat

bitter sparrow
#

so we're moving to gitlab?

#

let me start reading up on gitlab ci

slender belfry
#

bitbucket time

#

or just write commits into a book on a Minecraft server

#

GitHub back

tidal widget
slender belfry
#

Thanks for reviewing, @bitter sparrow 🙂

#

Dunno if you're still on it, but I addressed all of your comments.

bitter sparrow
slender belfry
#

@bitter sparrow I addressed all of your comments and made sure to point it out when you may want to add your review comment to the upstream PR as well

#

ah, forgot to do the one about mod-level documentation, sec

bitter sparrow
#

ok, i'll approve after that

#

not 100% satisfied, but good enough to merge and maybe iterate later

slender belfry
#

Is it alright for you if I do the game module cleanup now? It will probably cause merge conflicts in your PR, so I'll respect it if you say "no".

bitter sparrow
#

hm yeah i'll just update my pr

#

wait how do the buttons even work, without an OnPress component?

#

i think adding that component to the bundle was just missed

slender belfry
slender belfry
#

I'll add it to my current PR haha

bitter sparrow
#

same

slender belfry
#

So, about our cool "make the one-shot system a child of the button" idea @bright ledge @bitter sparrow

image.png
bright ledge
#

ah

slender belfry
bright ledge
#

maybe?

#

why are we doing it with a child out of interest

slender belfry
bright ledge
#

ah

slender belfry
#

We could also add an ugly SystemIdLink to the button and handle it in the on_remove hook.

bright ledge
slender belfry
#

That one never gets cleaned up by Bevy

#

So we need to ensure that somehow

bright ledge
#

who knew that having kids would be so complicated

slender belfry
#

Thanks for the addition to #1256006995656441898 message

bitter sparrow
#

bad warning, unsubscribed

slender belfry
#

I can just remove them on OnRemove, OnPress

#

That's fairly easy

bitter sparrow
#

but what if it's a button that doesn't cause the entire menu to despawn when you press it

#

like a button in a settings menu

slender belfry
slender belfry
bitter sparrow
bitter sparrow
slender belfry
slender belfry
#

At which point you cannot click the button

#

(I hope)

bitter sparrow
#

ah i read it as despawning inside the OnPress one-shot itself

#

the real problem here is that this warning is a false positive though?

#

i dont see why it's wrong to have a non-UI child of a UI entity

#

you can have a non-spatial child of a spatial entity right

bright ledge
#

I think we should also keep in mind that we may want some buttons to stay after they're pressed

#

if we ever add a settings menu I imagine we've want to keep the main menu

bitter sparrow
#

yeah, as long as we're only despawning the one-shot when the button despawns it should be ok

bright ledge
#

wouldn't we still want to keep the system though

bitter sparrow
#

would be re-adding it every time the button is spawned

bright ledge
#

ah

#

got it

bitter sparrow
#

we could avoid the memory leak without despawns by persisting the one-shot system, but that would require pre-registering them and storing them in a resource

#

which is a thing

slender belfry
slender belfry
slender belfry
#

oh wow this just panicked on my Windows machine on closing the window bavy

image.png
bitter sparrow
#

unwrap considered harmful. unwrap considered harmful. unwrap considered harmful. unwrap considered harmful. unwrap considered harmful.

slender belfry
bitter sparrow
#

let size = r!(window_query.get_single()).size() + 256.0;

slender belfry
bitter sparrow
#

also i'd rename that system to apply_screen_wrap now, just noticed

#

but yea

slender belfry
#

I sneakily sneakily bloated my PR with a fix for the panic and the warning

#

as a treat

bitter sparrow
#

great

#

i saw nothing

slender belfry
#

good idea

#

PR ready again

bitter sparrow
#

this ci failure is one of a few reasons i personally use one import per line c:

#

i recognize that it's not the common style in the rust ecosystem tho, and not the rustfmt default

slender belfry
#

is it?

bitter sparrow
#

nightly rustfmt has some options

#

in my rustfmt.toml:

# Unstable
group_imports = "StdExternalCrate"
imports_granularity = "Item"
imports_layout = "HorizontalVertical"
match_block_trailing_comma = true
normalize_doc_attributes = true
slender belfry
bitter sparrow
#

understandable

slender belfry
#

Sometimes, my style preferences are truly just vibes based

#

I'll update the design doc and readme now

#

Your PR is the only thing missing from Alice's feedback issue 🎉

bitter sparrow
#

working on the merge conflicts

#

i get a crash in despawn_one_shot_system when i hit play

#

oh wait

#

that's because i am both childing the system and now manually despawning

#

not better like this imo but it's w/e since i have my own template :p

#

oh i forgot to replace the In<(Entity, Self)> stuff, will do that now EDIT: pushed

slender belfry
bitter sparrow
#

you have to use SystemState to access system params that aren't queries or resources, and then you have to clone everything you'll need out of them before doing world.entity_mut... for lifetime reasons

slender belfry
#

I mean, it's not hard or anything

bitter sparrow
#

it's not just custom SystemParams, it's anything world doesn't happen to support

#

or supports but not ergonomically

#

3rd-party libraries may also provide SystemParams

#

calling world.resource::<Xyz>() a bunch of times at the top of the function to avoid SystemState isn't ergonomic either anyways

#

anyways like i said i'm not too concerned about this personally, because i know that in my own code i won't have to deal with these shenanigans

#

if we think this is better for bevy_quickstart's target audience then it's fine

slender belfry
slender belfry
slender belfry
slender belfry
bitter sparrow
#

also if you're calling world.resource_scope with a closure inside...

#

at that point you could just call world.run_system_once_with((id, self), closure_or_fn) and be happy :p

slender belfry
#

Btw, I left you some review comments. Nothing big.

#

Really curious to hear @prisma delta's feedback (this is the last task from your feedback list that has not yet been addressed and merged FYI)

slender belfry
slender belfry
slender belfry
slender belfry
prisma delta
bitter sparrow
#

alright well we have until bevy 0.16 to improve things upstream anyways

#

ig there's no adequate solution that can also be considered "simple enough for beginners" in bevy 0.14

slender belfry
slender belfry
#

Bit unfortunate for the work, sorry @bitter sparrow

prisma delta
#

Yeah. I think we can get there, and I think this is a step in the right direction

slender belfry
bitter sparrow
slender belfry
#

For the moment, I guess we go back to the variant where we don't return anything from spawn, but use a command instead of an observer?

bitter sparrow
#

if it's not viable, then spawn_with + impl EntityCommand for functions that take Entity as their first arg

bitter sparrow
#

i think that might not work in all the ChildBuilder / World / etc. variants though

slender belfry
bitter sparrow
#

yeah i think so. that's what spawn_with does internally. also spawn(bundle) = spawn_empty().insert(bundle)

prisma delta
#

We have enough folks here that we should be able to review and merge it easily

slender belfry
prisma delta
#

I really prefer construct_player

slender belfry
bitter sparrow
slender belfry
bitter sparrow
#

want to make sure i'm not doing something that will be redundant, or something that could be better

slender belfry
#

I'll do a PR now for the crappy but simple version of entity spawning

#

Btw, could you take a look at my 2 open PRs?

#

Or would you prefer if I used a struct SpawnPlayer that the user then manually adds?

#

Btw, thoughts on SFX vs sound effects? I renamed them to the verbose variant a while ago, but it's starting to annoy me, so I'd like to go back to SFX

bitter sparrow
#

soundtrack is verbose too, i like music better 🤐

slender belfry
#

I'll do a rename with both

bitter sparrow
#

even though i will admit "soundtrack" is technically slightly more accurate

slender belfry
#

I think readers will understand

#

hmm, what about the assets/soundtracks dir? assets/music?

slender belfry
bitter sparrow
#

ok. i mainly feel like it's confusing that we now have this layer that maps path -> handle, when that's the same thing that AssetServer does

#

since previously it was mapping key -> handle which is different

#

also we could rename key -> path if we're actually committing to always using path as key (which we may not be?)

prisma delta
slender belfry
prisma delta
#

Extension traits are nice, but they're another layer of complexity just for some ergonomics

bitter sparrow
#

what about the extension traits we're using for UI / audio?

#

i think the UI ones are worth it at least

slender belfry
#

I'd keep them both.

slender belfry
slender belfry
prisma delta
prisma delta
slender belfry
bitter sparrow
slender belfry
#

Rest of the PRs should also be ready

#

@prisma delta once we get the currently open PRs merged, we are ready for another round of feedback. Would you prefer taking a look at it on your own again or should I ping the other Bevy maintainers as well this time?

prisma delta
slender belfry
#

Also @sinful hull since you've been following a bit along and are imo exactly in our target demographic of "wants to create games using Bevy in a simple way", feel free to tell us if we've been making anything worse through these updates, or if there's anything big missing, or we're missing documentation anywhere, etc.

prisma delta
#

(or if things feel confusing and over-engineered anywhere!)

slender belfry
slender belfry
sinful hull
# slender belfry Also <@208800833700102144> since you've been following a bit along and are imo e...

As I mentioned over in #general the template was really great for the jam, thanks for all your hard work.

One thing I'm may be not 100% clear on is the liberal sprinkling of #[derive(Reflect)]. As far as I can tell this isn't required, and there isn't any info in the PR for why this was necessary. During the game I experimented removing it from a few places and... nothing happened? 🤷‍♀️

slender belfry
sinful hull
#

ah yeah ok thanks. I think design docs might be better, its probably in too many places to comment against each usage?

prisma delta
hidden pond
#

BTW @slender belfry have you started work on the survey at all yet?

slender belfry
hidden pond
#

Fair enough, just thought I would ask

#

I'll try and get started this weekend if you don't get to it before me.

slender belfry
#

Also, is there anything I can do to help the other two PRs get along?

slender belfry
#

@bitter sparrow ready again. Btw, I'm tending towards renaming theme to ui_tools again, due to stuff like interaction being in there. Thoughts?

bitter sparrow
#

i think theme is still better

#

there's InteractionPalette which is visual and OnPress is the outlier

#

but i wouldn't rename it for just the one outlier

slender belfry
#

should I maybe move it out?

bitter sparrow
#

we don't have a util/

#

if we did then maybe

slender belfry
#

yeah, I can see how a future review will be unhappy about a module called ui_tools only containing one submodule

#

meh

#

You're right, I'll leave it then.

bitter sparrow
#

i need to figure out a way to rationalize how OnPress is actually theming

#

maybe you play a sound effect on press 👍

slender belfry
slender belfry
#

Well, trigger_interaction_sfx is already in there

#

And apply_interaction_palette

#

which are both definitely theming

#

Just the callback stuff is iffy

bitter sparrow
#

behavioral theming

slender belfry
#

@bitter sparrow good idea. I'll do a follow up in a few mins. How about having a parent field on SpawnPlayer so that we can insert it as a child of Level?

image.png
#

That way, I can also remove the comment about expanding the struct

bitter sparrow
#

oh right

#

we can't just add it as a child lol

#

i forgot

slender belfry
bitter sparrow
#

honestly in that case i'd rather not spawn a parent level entity 🤐

slender belfry
#

But spawning something as a child will come up, so we should have an answer for it.

#

Or we just shrug and point at Bevy?

bitter sparrow
#

yea imo

#

it should be fixed by 0.16

slender belfry
#

Hmm fair enough

#

I'll leave it as-is then

#

Something rubs me the wrong way about changing the screen/playing.rs plugin to just spawn the player directly

#

sounds like an ugly implementation detail leak

bitter sparrow
#

i mean personally i think it's totally reasonable

#

you spawn the HUD, the player, the enemies (or a "wave" entity that spawns enemies periodically or w/e)

#

possibly delegating some of that logic to other modules that want to take responsibility for it

#

so a level.rs is fine but i don't think it's conceptually required

slender belfry
#

Hmm, I see your point

#

oh whaa

#

TIL

bitter sparrow
#

funny

#

another option would be to actually include some little bit of data in there

#

and then the comment can be removed completely

#

like "scale" since we 8x the sprite

slender belfry
#

I can add the max speed

bitter sparrow
#

sure

#

anything works

slender belfry
#

Also fixed a naming inconsistency

#

@bitter sparrow

Might need to do world.flush_commands() after this
Isn't that stuff done completely automatically nowadays?

bitter sparrow
#

between systems with system ordering yes

#

i dont remember the exact reason but i had a bug that was fixed by adding it here:

impl AddExt for EntityWorldMut<'_> {
    fn add<M: 'static>(&mut self, command: impl EntityCommand<M>) -> &mut Self {
        let id = self.id();
        self.world_scope(|world| {
            world.commands().add(command.with_entity(id));
            world.flush_commands();
        });
        self
    }
}
#

during the jam

slender belfry
#

Oh, you mean so that we actually apply the inner command right now?

bitter sparrow
#

i think it's for an ordering reason (relative to other commands) yeah

#

but i'm not 100% sure

#

would be nice to have world.run_command(x) == world.commands().add(x); world.flush_commands() maybe

#

idk it's another api diff

slender belfry
#

BTW our CI is having a bit of a struggle

#

I liked your suggestion @bitter sparrow, but I really struggled to implement them without a lot of ugly boilerplate

#

I think asset boilerplate can quickly become a particularly nasty part of Bevy in general

bitter sparrow
#

i agree. the asset system is too low level

slender belfry
#

Not as bad as rendering boilerplate; thank god we don't have any of that.

bitter sparrow
#

or rather there is no high level built on top of it provided by the engine

#

something like loading a .ron that links to other assets, loading those, and then being able to track all of that in a loading screen

#

the asset system is capable of that but you really have to know what you're doing, and there's gonna be boilerplate

#

(i literally have this set up in my jam template, but i'm using iyes_progress and it's still pretty manual :p)

slender belfry
#

Completely agree with the reasonableness of that system

slender belfry
#

I think at this point my vision for Foxtrot 0.14 might just be your template, but in 3D

bitter sparrow
#

it's pretty useful. i think it could prob be improved a bit more

bitter sparrow
slender belfry
#

(And with Yarn Spinner integration)

bitter sparrow
#

foxtrot is a lot more featureful

#

for now

slender belfry
#

The features are the easy part

#

The high-level organization is the hard part, imo

#

(that and some crate-specific hacks)

bitter sparrow
#

template convergence is cool though, especially if the main differentiating factors are what the templates are optimized for

#

like game jams vs 3d games vs pixel art etc

slender belfry
bitter sparrow
#

the underlying patterns don't have to be unique each time

slender belfry
slender belfry
bitter sparrow
#

yeah, even better if the patterns dissolve into bevy 😄

#

ok i was gonna check the ci first

#

it seems the Tests job is taking 12m each time, but it is hitting the cache

slender belfry
bitter sparrow
#

not even restoring from a fallback cache, it's just a hit

slender belfry
bitter sparrow
#

i suspect we're pulling in a new version of cargo-cache with the same @v2 tag

#

nvm there has not been a recent release

#

huh

#

so checking the first CI run where it was slow and the one immediately before

#

we went from cargo 1.80.0 to cargo 1.80.1

#

but it seems like cargo --version is returning the same output?

#

wild

# 
cargo 1.80.0 (376290515 2024-07-16)
cargo 1.80.1 (376290515 2024-07-16)
#

we can manually clear our cache as a workaround to repopulate it EDIT: done

bitter sparrow
slender belfry
bitter sparrow
#

ok, in that case also rename let files = to let paths =

#

also the design doc will need to be updated

slender belfry
#

done 🙂

bitter sparrow
#

ah you did it

slender belfry
#

I was anticipating a review comment of "why do a whole iterator when it's just one entry" down the line

image.png
#

But I personally agree

bitter sparrow
#

yeah the iterator separates the boilerplate from the data

slender belfry
#

Also, this way we make it a bit more explicit that the Ducky in particular needs the pixel art settings

#

The iterator approach would imo require a pixel_art_paths and such

bitter sparrow
#

or a triple

#

but in my mind we would apply the pixel art settings to every image :p

slender belfry
bitter sparrow
#

oh i mean it would be a pair in the original list

#

and then it'd still be a pair after so technically not a triple

slender belfry
#

I also had this as an iterator at some point and reverted it

image.png
bitter sparrow
#

yeah in my own code that would definitely be an iterator

slender belfry
slender belfry
slender belfry
#

tbf I'm an iterator supremacist

slender belfry
#

@prisma delta all PRs are merged now, we've addressed all of the concerns you noted the last time, we have addressed some feedback by others, and we updated the readme and design doc accordingly.
We're ready for another first pass from you 🎉

prisma delta
slender belfry
slender belfry
bitter sparrow
slender belfry
#

I think our cache is somehow out of date?

#

I'll push a trivial change to Cargo.toml to force invalidation

#

hrrmmmm why is it still receiving a cache?

#

Eh, I'll just accept it for now.

#

It is what it is.

bitter sparrow
#

seems like it was fine

#

it pulled the cache from a different job, so it's a fallback not a hit

willow remnant
#

@bitter sparrow @slender belfry re https://github.com/TheBevyFlock/bevy_quickstart/pull/242, I'm trying to work backwards to find the ultimate reason "we should use commands not observers here" and I can't seem to find the reason (there are a lot of links to links so maybe I missed it).

I know it was from Alice's feedback:

I'm not convinced that spawning things like the player should be an observer; that seems like a pointless layer of indirection over just using a custom command

But I'm not really clear on the why in this feedback. Is the pointless indirection to do with performance? Or with ergonomics?

Related to the linked merged PR, it's not clear to me from the comment attached to world.flush_commands(); why this needs to be done, or what happens if this wasn't done.

All in all, if this is the recommended official approach, that's great and I'm glad to see it rather than being lead astray with bad observer patterns, but also it looks scarier than what was there previously with "simple observer code".

slender belfry
# willow remnant <@231551294060494849> <@227474830470021130> re https://github.com/TheBevyFlock/b...

(@prisma delta , correct me if I'm wrong here)
The point is that the observer in question will trigger a command, which means that the flow of control looks like this:
gameplay system -> triggers an observer -> adds a command
by using a command directly, we simplify this to
gameplay system -> adds a command

Now there's a caveat (as far as I understand, I'm not entirely 100% certain on this one). If we spawn the level, we call another command in there, so the flow becomes:
gameplay system -> adds a command -> adds a command

flush_commands applies all commands added to the queue, so it becomes
gameplay system -> adds nested commands and executes them all

I personally agree that observers are preferable in this case because of better ergonomics (and I don't care about the indirection), but multiple experienced Bevy users reported something along the lines of "I'm confused why you would use an observer there; why not just a command?", so we changed it.

prisma delta
slender belfry
#

Interestingly, you are the second person to report that they find the new way more scary than the observer way, so this may be something to reconsider

prisma delta
#

I think exclusive world access is scary 🙂

slender belfry
#

Maybe there is a divide there between how seasoned Bevy users and newer users see things? How would you describe your skill level, @willow remnant ?

slender belfry
#

That's actually a good question. @sinful hull and @willow remnant, can you describe what exactly feels worse in the command approach?

willow remnant
slender belfry
willow remnant
slender belfry
slender belfry
slender belfry
willow remnant
#

I have no idea if it's out of context to share my code, but the first pass of this template with observers looked great to me and I naively followed it (which is fine). I ended up with a bunch of systems/plugins looking like this:

app.observe(setup_player1_army);
app.observe(setup_player2_army);
app.observe(spawn_regiment);
app.observe(auto_deploy_regiment);
app.observe(auto_deploy_player1_regiments);
app.observe(auto_deploy_player2_regiments);
app.observe(deploy_regiment);
app.observe(spawn_unit);
app.observe(change_regiment_ranks);

app.add_systems(
    Update,
    (
        (|mut commands: Commands| {
            commands.trigger(AutoDeployPlayer1Regiments);
        })
        .run_if(input_just_pressed(KeyCode::KeyA)),
        (|mut commands: Commands| {
            commands.trigger(AutoDeployPlayer2Regiments);
        })
        .run_if(input_just_pressed(KeyCode::KeyJ)),
        (|mut commands: Commands| {
            commands.trigger(ChangeRegimentRanks::Increase);
        })
        .run_if(input_just_pressed(KeyCode::Equal)),
        (|mut commands: Commands| {
            commands.trigger(ChangeRegimentRanks::Decrease);
        })
        .run_if(input_just_pressed(KeyCode::Minus)),
    )
        .run_if(in_state(BattleState::Deployment)),
);

I guess you could say I did "observers all the way down" and, since seeing the updated template, it just made me wonder "oh wait, did I abuse observers, should I use commands for some of this stuff"

slender belfry
willow remnant
willow remnant
# slender belfry Also, could you take a look at [this PR](https://github.com/TheBevyFlock/bevy_qu...

I think much of a muchness. If there was a reason to be able to call play_soundtrack in multiple places, I guess that makes more sense. But really world.run_system_once_with(self, play_soundtrack); does itself feel like pointless indirection at a glance even if it's done to keep the system as a neat separate func. Probably just a personal nit but run_system_once_with as a name just looks ugly. I don't have a better naming suggestion so that's not super constructive.

slender belfry
slender belfry
# willow remnant Yeah more noisy. E.g. before observers, I wrote a couple of one-shot systems and...

I think it's a bit early to say what is "right" or "wrong" for observers, outside of very clear cases.
Some users, as far as I can tell, are using the following principles:

  • custom command for when they expect to use commands in their systems (as in bevy_quickstart)
  • observers when they want to mutate existing components, react to OnAdd and friends, or need bubbling behavior
  • hooks for when a hierarchy needs to be built
  • events for when stuff needs to be parallelized or handled in a specific schedule

I personally am currently using observers for nearly all of these, pretty much the same way as you, as I really like the ergonomics so far and don't care much about the "purity" factor behind it or the minor performance impact. This may very well change in the future, but that is my current leaning.

willow remnant
# willow remnant I have no idea if it's out of context to share my code, but the first pass of th...

Actually, might be useful to share: But what led me down the path of checking if commands should be preferred to observers.

I was debugging why one of my observer functions can't find a certain UI entity from a query. I can see through my logs that the UI entity is spawned, then I trigger a certain observer, and that runs immediately and it fails to find the entity. No need to debug this here, but assuming I don't have a silly bug I just wondered if I shouldn't be using observers for this, rather I need to use commands to have them applied before my observer func can successfully run and query for that entity.

slender belfry
slender belfry
bitter sparrow
#

commands should be used for something that "performs a simple operation on the world"

#

which is what we're doing when we spawn something

willow remnant
bitter sparrow
#

events should be used for something that happens, and we want to react

#

the fact that commands are less ergonomic to write than observers is a bevy issue

slender belfry
#

And further blur the line between observers and commands 🙃

bitter sparrow
#

well observers effectively means running an entire schedule when an event occurs

#

commands do one thing

slender belfry
bitter sparrow
#

yeah that's a more general bevy issue where there's a proliferation of ecs tools with overlapping use cases

slender belfry
bitter sparrow
#

#ecs message

slender belfry
bitter sparrow
#

i also feel like observers ought to support run conditions and system ordering

#

aka order between two observers listening for the same event

#

rn you just can't

slender belfry
bitter sparrow
#

at which point it will look even more like a schedule, but using an event as the schedule label instead

slender belfry
#

(At least they support piping in 0.14.1)

bitter sparrow
#

oh interesting

slender belfry
bitter sparrow
#

observers are basically "run a schedule with an input", which you could also do by setting a temporary resource with the value of the input

slender belfry
#

Anyhoot, @willow remnant, is there anything else that caught your eye as overengineered or confusing?

#

Again, I'm really interested in your feedback here 🙂

bitter sparrow
#

i mean i fully agree that World access absolutely sucks compared to systems

#

it feels like dropping down to unsafe

slender belfry
#

Yeah, I think we are all in agreement about the following:

  • Commands are conceptually the right tool for this
  • Observers have the better ergonomics
bitter sparrow
#

and improving World ergonomics upstream would be tough because of lifetime issues

#

which systems avoid by (behind the scenes) removing the particular things you're interested in from the World then putting them back in iirc

prisma delta
#

By giving out multiple mutable pointers to disjoint parts

bitter sparrow
#

ah okay, ig they would need to for parallelism

slender belfry
willow remnant
# slender belfry Anyhoot, <@1113668290695413770>, is there anything else that caught your eye as ...

I used the version around the time the jam started (I didn't finish my entry) and it was a welcomed template and I found it really useful. Not to harp on, but when I saw the patterns for spawning stuff with observers I was like "oh cool that makes sense, I love that", and that was the stand out for me. As above, I'm glad to iterate on it and adjust based on official best practices, so it's all good if that initial happiness goes away for the sake of doing things a better way.

I've seen the changes to SFX, E.g. now it's commands.play_sound_effect(SoundEffectHandles::PATH_BUTTON_PRESS). Maybe a nit: but I actually think you should just use sfx, seems better and SFX seems like a pretty universal term in game dev, no? It would become: commands.play_sfx(SfxHandles::PATH_BUTTON_PRESS). I think the PATH_ thing looks a bit odd compared to before. I could actually see you ditching the PATH_ prefix actually. The Handles in the name implies a path in some ways so it may be redundant. So it could go to commands.play_sfx(SfxHandles::BUTTON_PRESS).

prisma delta
#

FYI I'll do a pass on the template again tomorrow, and see if there's anything I can simplify

bitter sparrow
slender belfry
#

I'll do that right now haha

bitter sparrow
#

the SfxHandles::BUTTON_PRESS vs SfxHandles::PATH_BUTTON_PRESS is due to a concern that the former reads like you have an actual Handle

#

when it's actually a string with the file path to the asset

#

it is wordier this way. it would read best if it was SfxPaths::BUTTON_PRESS somehow

slender belfry
bitter sparrow
#

yep

slender belfry
willow remnant
slender belfry
bitter sparrow
#

true. imo "soundtracks" is technically slightly more accurate than "music", because it specifies that it's the main music that plays in the background

#

but i don't think it's worth the extra 6 characters

slender belfry
#

So I opted to use "soundtrack" in comments and "music" in the code

#

But then I realized the path on disc was "assets/soundtracks" and that made more sense to me than "assets/music"

bitter sparrow
slender belfry
slender belfry
#

I like how ImageHandles is explicit about holding handles and not images though

#

ImageMap sounds a bit like it's holding images

bitter sparrow
#

that's true

slender belfry
#

I'm a bit cautious here because I've seen beginners struggle with handle vs asset a lot

#

so I want to be extra explicit on that

bitter sparrow
#

i just like that bgm is the same length as sfx

willow remnant
#

Minor thing, not sure if it was just me, but I seemed to need to add "rust-analyzer.check.command": "clippy", to my .vscode/settings to match what CI was checking. I think I pushed a change to GH, clippy failed but there was no issue in my VS Code until I added this setting to make the template also use clippy. I was going to make a PR but wasn't entirely sure if it's the right call or just a me thing.

slender belfry
bitter sparrow
#

bgm passes the duckduckgo test as well. the results are all background music

slender belfry
#

I kinda like having long names on disk and short names in code. Thoughts on that?

#

Because bgm and sfx kinda blur together when I see two directories like that next to each other

#

But in code, these are never in close proximity

bitter sparrow
slender belfry
slender belfry
bitter sparrow
#

it's more expensive to run + it doesn't always catch actual errors, often it's like style stuff that you can fix later

bitter sparrow
willow remnant
# slender belfry I kinda like having long names on disk and short names in code. Thoughts on that...

I think I'd personally just use the same thing in code and file names. Again, SFX is universal so if I saw assets/sfx I would know what it means. If I see assets/bgm and I don't know what it means, maybe it indicates BGM isn't a universal enough term and so is a bad choice. But yeah, I'm impartial here. If I had to pick, I'd probably go with "bgm" and in code I'd just have a comment on the main type saying "Background music" so that users have a way to learn if they are confused. Anyway, I defer to more experienced people here.

bitter sparrow
#

i'm personally fine with either "music" or "bgm"

#

but at the very least the module name has to match so it would be music.rs or bgm.rs

slender belfry
#

I'll do a separate PR for SFX and BGM

slender belfry
bitter sparrow
#

that could make a good jam game

#

the directory names get shorter and shorter each level

slender belfry
#

Huh, our SFX dir is already called sfx

#

Apparently it was not renamed to sound_effects

#

Here's the SFX rename

bitter sparrow
slender belfry
#

hurray, the cache works again

slender belfry
slender belfry
# bitter sparrow

Ah, I see what you mean. I was referring to assets/sfx 🙂 nvm then

bitter sparrow
slender belfry
bitter sparrow
#

way better for autocomplete too

slender belfry
#

do you mean the comment?

bitter sparrow
#

the trait methods

slender belfry
#

Aaaaah

#

thanks

#

haha, completely glossed over it

bitter sparrow
#

do you have / use ripgrep?

#

i would do like rg -i 'sound_?effect' then ctrl+click the linked files

#

well this is a small refactor so i might not have done that here

slender belfry
#

Usually do the same, but in the regex find/replace window of VS Code

#

But yeah, also didn't think of it here 😄

#

there, fixed

willow remnant
#

Two missed occurrences SoundEffectCommands, SoundEffects (sorry didn't get to review in time)

bitter sparrow
#

can throw that in with the soundtracks rename

slender belfry
slender belfry
bitter sparrow
#

this reminds me of my Ssa = SpritesheetAnimation heh

#

except that one is definitely not standard

willow remnant
bitter sparrow
#

hm bevy has it as SpriteSheetBundle

slender belfry
bitter sparrow
#

sosa

#

some words are simply too long. it's why we have i18n and a11y

slender belfry
#

s18a

bitter sparrow
#

perfect

slender belfry
#

I somehow have a feeling that BGM is a bit too rare to pass future reviews and we'll have to revert, but I'm ready to throw those dice cooltofu

bitter sparrow
#

ah auto-merge

#

nevermind then

bitter sparrow
#

soundtrack -> bgm -> music 🤝

willow remnant
#

i think there are some missed cases, follow up? IsSoundtrack, PlaySoundtrack, soundtrack_handles, soundtrack_query, soundtrack_key

slender belfry
willow remnant
#

You'll get there for the 3rd thing I make you rename.

bitter sparrow
#

naming things is the hardest problem in programming

#

so it stands to reason that renaming is just as hard

slender belfry
slender belfry
slender belfry
#

The only logical next step is to then rename asset to ass, for the three letter consistency

bitter sparrow
#

ui -> gui

slender belfry
#

bev

bitter sparrow
#

nah it's bvy this has already been done

slender belfry
#

at least src, lib, and mod are already there

slender belfry
bitter sparrow
slender belfry
#

Ah, there we go

image.png
bitter sparrow
#

i love that it's still using query.iter_mut() instead of &mut query in the sample code

slender belfry
#

hehehe

bitter sparrow
#

what's the point of #[reflect(Debug)] btw?

prisma delta
slender belfry
#

@bitter sparrow is it possible that your PR is using "native" VS Code snippets while I'm talking about dedicated rust-analyzer snippets supported in the rust-analyzer.completion.snippets.custom key?

bitter sparrow
#

yes i believe so

#

rust-analyzer has its own little snippets implementation it seems

slender belfry
bitter sparrow
#

because vs code wouldn't know how to make "requires" work in rust (or expression-scoped postfix snippets for that matter)

#

do we want to use rust-analyzer snippets for this?

slender belfry
#

Pretty sure that would be better, yeah

#

I'm off to bed now, but I'll take a look at what you come up with when I wake up 🙂

bitter sparrow
#

alright cya

lament horizon
# bitter sparrow what's the point of `#[reflect(Debug)]` btw?

It basically makes the Debug output of a dyn Reflect use the concrete type's Debug impl. In general, there shouldn't be much of a difference in the output. The main difference comes when a type (or one of its nested types) has either a custom Debug impl or when some of its fields are marked #[reflect(ignore)]

#

There's even a PR to deprecate the attribute and have it automatically use Debug if available , but personally I'm hesitant to add autoref specialization magic to bevy_reflect

sinful hull
# slender belfry That's actually a good question. <@208800833700102144> and <@1113668290695413770...

For me the code seems less readable and ergonomic. The previous mental model of "triggering a spawn" feels more in line with what I'm trying to do.

I think as I said elsewhere the code here feels more indirect even though under the hood it's the opposite.

Warning personal opinion: For things like this that aren't on the hot path I guess I'd usually favour readability. Having said that in a game jam id likely be happy to keep whatever's in the template so I don't feel like it's a big deal.

I've been using bevy since 0.3 (I think?) mostly sporadically for game jams. I don't think I've ever written a "world" system FWIW.

And now I am very late for something so I should get off discord 🤣

lunar sail
bitter sparrow
#

is it just 1.80.0 + rustc version bump?

lunar sail
#

That's what I'm assuming, that it always matches the Rust version, but the commit hash and date is of the cargo crate itself

bitter sparrow
#

i was expecting the fix to be to include the entire cargo --version string which would catch the 1.80.0 vs 1.80.1 diff

lunar sail
bitter sparrow
#

ic

lunar sail
#

What would you think if I keep the part about including rustc --version, but remove the part that only includes the part in paranthesis?

bitter sparrow
#

so the full cargo --version and the full rustc --version?

lunar sail
#

Then we should be double safe

#

Yea

bitter sparrow
#

it sounds reasonable to me on a surface level, but it really depends on how exactly those strings change

#

i'll do a little bit of digging first

lunar sail
#

This whole caching thing is so cursed, I never thought that we'd run into so many edge cases when I started setting up the action lol

bitter sparrow
# 
$ cargo +stable --version
cargo 1.80.0 (376290515 2024-07-16)
$ rustc +stable --version
rustc 1.80.0 (051478957 2024-07-21)

$ rustup update

$ cargo +stable --version
cargo 1.80.1 (376290515 2024-07-16)
$ rustc +stable --version
rustc 1.80.1 (3f5fd8dd4 2024-08-06)
#

just ran this on my own pc

#

it seems like the version number is intentionally in lockstep even though cargo itself didn't have a point release

#

but rustc did have a point release yesterday

#

honestly could probably get away with only using rustc --version hash

#

actually idk. would they do an empty rustc point release to fix a bug in cargo?

#

but anyways since they update together, i don't think it'll cause unnecessary cache invalidations

#

it might just make the cache key more verbose, which is probably fine

bitter sparrow
#

ok my conclusion is we want both rustc and cargo hashes but not the version numbers

#

which is what the pr already does :)

bitter sparrow
slender belfry
slender belfry
#

@bitter sparrow @bright ledge today, @limber dragon reminded me that calling .observe directly on EntityCommands was a thing, which I totally forgot.
I changed our little OnPress pattern to use that instead: https://github.com/TheBevyFlock/bevy_quickstart/pull/249

GitHub
Use observers for callbacks by janhohenheim · Pull Request #249 · T...

This variant is probably closer to what will be upstreamed in Bevy
Its implementation is simpler (no need to learn about one-shot systems or SystemId, no need to be careful about memory leaks)
I re...

bright ledge
#

ooh

prisma delta
bright ledge
#

I think it might be worth pointing out the SystemId method because there are some things that the observer pattern can't do but otherwise this is really cool

#

also thanks for pointing it out

slender belfry
agile dirge
#

i like that a lot

bright ledge
#

I likely wouldn't have found it otherwise

bright ledge
#

and in my case I don't think you can store them like SystemId's

slender belfry
bright ledge
#

huh

#

nvm then

#

are you able to store them for later?

agile dirge
#

I'm pretty sure running systems manually returns the system output though?

bright ledge
#

yes

agile dirge
#

I'm missing the point then

bright ledge
#

that's something I use to return the next bit of dialogue from my buttons actions

bright ledge
agile dirge
#

ah, i see

slender belfry
# bright ledge are you able to store them for later?

Only if you add that to a resource or something:


fn spawn_button(mut commands: Commands) {
    commands.button("talk").observe(hello.pipe(store_dialog));
}

fn hello(_trigger: Trigger<OnPress>) -> String {
    "Hello there".to_string()
}

fn store_dialog(In(dialog): In<String>, mut dialogs: ResMut<Dialogs>) {
    dialogs.append(dialog);
}
bright ledge
#

ah no I mean store the observer

#

like with SystemId's

agile dirge
#

observers are entities

#

they have Entity

bitter sparrow
#

does the observer entity despawn when the button entity despawns?

slender belfry
limber dragon
agile dirge
slender belfry
agile dirge
#

and it still technically has Entity

bright ledge
#

it also won't work in my case because I need to spawn buttons after I create the system

slender belfry
limber dragon
#

.observe() does create a new entity, but you can also technically use Observer directly (edit: but you probably shouldn't, see Mini below)

bright ledge
#

I think

bitter sparrow
bright ledge
#

I'm not entirely clear on under the hood becy

slender belfry
#

I'll investigate

agile dirge
#

which most users dont

bitter sparrow
#

i wonder if you could attach the one-shot system's components to the button entity itself too

limber dragon
agile dirge
limber dragon
bitter sparrow
#

it's funny seeing despawn and knowing it means your game will crash if you add a parent or child to that entity 😄

slender belfry
agile dirge
#

that reminds me, observer API still doesn't return the Observer entity in some(?) cases

#

i dont remember the exact issue, but it was pretty inconsistent

slender belfry
bitter sparrow
#

there's a diagnostic plugin for entity count also

slender belfry
agile dirge
#

who needs valgrind

bitter sparrow
slender belfry
agile dirge
#

I really like how template evolved and now provides a very nice feedback to a lot of Bevy API

#

this template for sure makes it to the bevy ui

slender belfry
#

@agile dirge I'm struggling to implement a function returning a closure according to your suggestion. Could you help out?

agile dirge
#

sure

slender belfry
#

thx 🙂

agile dirge
#

let my update my template and ill see if i can get it working

dawn crag
agile dirge
# 
fn change_state<S: FreelyMutableState>(
    new_state: S,
) -> impl Fn(Trigger<OnPress>, ResMut<NextState<S>>) {
    move |_trigger, mut next_state| next_state.set(new_state.clone())
}
#

i was 🤏 from having it the exact same line footprint as original

#

and then i had to add move and it reformatted itself

#

I'm still compiling to see if it doesn't break at runtime

bright ledge
agile dirge
#

clone is necessary because it's not an FnOnce

bright ledge
#

ah

agile dirge
#

move moves the original into the closure data

#

.clone() makes a new copy for each execution

bright ledge
#

got it thanks

slender belfry
agile dirge
#

ah right, I forgot you weren't in the state working group

#

Pyrious was there xD

slender belfry
bitter sparrow
#

should just be one type of State

agile dirge
#

yeah

bitter sparrow
#

will get there eventually

agile dirge
#

yup

slender belfry
agile dirge
#

I'll have more time again starting second half of next month

agile dirge
strong relic
slender belfry
#

I feel like we have more freedom there.

agile dirge
#

sure

slender belfry
agile dirge
#

so it's fair to assume Entity is an identifier for the observer

#

like actually actually, not a coincidence

strong relic
#

Yep, ideally every place we need a unique identifier it should just be Entity, a lot of progress is being made in that direction

agile dirge
#

that mostly implies everything as entities, which I'm all for

#

how about ComponentId is that also going to change to Entity?

bitter sparrow
#

components as entities is a thing, flecs has it

strong relic
#

There could still be a ComponentId type but it would just wrap Entity

#

Just one step before relations

agile dirge
#

I'm not sure if the implication of "unloading" a component is something we want

#

but holes like this can be said for everything basically

strong relic
#

It needs to exist for relations to exist, not to get too offtopic for this wg when we have a relations one

#

Since a pair of (ChildOf, entity) can be added effectively as a component and will stop being valid when entity is despawned

#

But yes in general you shouldn't be despawning components

slender belfry
agile dirge
#

can't wait for new ui to land so we can replace Interaction

bitter sparrow
#

buttons that act on mouse down 🤢

agile dirge
#

hover and press are disjoint but they're mutually exclusive with Interaction 😫

slender belfry
#

@bitter sparrow I addressed your comments

bitter sparrow
unreal narwhal
#

fyi signing for macos is going to get more important, probably before next jam: https://developer.apple.com/news/?id=saqachfa

Updates to runtime protection in macOS Sequoia - Latest News - Appl...

In macOS Sequoia, users will no longer be able to Control-click to override Gatekeeper when opening software that isn’t signed correctly or notarized. They’ll need to visit System Settings > Privacy & Security to review security information for software before allowing it to run.If you distribute software outside of the Mac App Store, we recomme...

dawn crag
#

I could have been control-clicking this whole time?!

unreal narwhal
slender belfry
slender belfry
slender belfry
unreal narwhal
#

I noticed the windows builds from the template also came with runtime warnings

slender belfry
#

Bleh

#

I hope some CI and packaging wizard swoops along to fix it

#

That stuff is so annoying to fix

slender belfry
slender belfry
#

I know I have fixed it by creating an app installer in Foxtrot, but that is such a huge amount of config lurking in some deeply nested directory

#

I wouldn’t want to replicate that if possible

bitter sparrow
#

ok so rust-analyzer snippet isn't gonna work

#

there are three supported scopes: expr, item, and type

#

the default scope is expr

#

none of the scopes support top-level code like defining a type outside of a function

slender belfry
#

Hmm? But how does my plugin snippet work then?

bitter sparrow
#

which plugin snippet

slender belfry
#

Wait

# 
 "rust-analyzer.completion.snippets.custom": {
      "plugin": {
          "prefix": [
              "plugin"
          ],
          "body": [
              "pub(super) fn plugin(app: &mut App) {",
              "",
              "}"
          ],
          "requires": "bevy::prelude::*",
          "description": "Create a new Bevy plugin",
          "scope": "item"
      },
 }
#

(I omitted the rest of the default snippets)

slender belfry
bitter sparrow
#

okay item scope works i think, vs code / rust-analyzer is really bad about updating to a new version of the snippet when settings.json changes

#

have to fully close and reopen IDE to make sure it updates

#

restarting rust-analyzer extension is not sufficient

#

changes to bevy.code-snippet were processed immediately on save otoh

#

i get a warning that the snippet is invalid after adding "requires": "bevy::prelude::*",

slender belfry
#

brilliant

bitter sparrow
slender belfry
bitter sparrow
#

weird

slender belfry
#

I think it's fine to use the regular snippets then

bitter sparrow
#

well why does it work for you but not me though

slender belfry
#

is it a false warning?

#

I'm on macOS right now, but I remember getting a similar warning on Windows, even though everything works fine

bitter sparrow
#

nah it doesn't work

slender belfry
bitter sparrow
#

that doesn't seem to be using requires

slender belfry
bitter sparrow
#

are you sure your vs code isn't using an old version of the snippet?

slender belfry
bitter sparrow
#

try changing the description and reloading it?

#

hmm

slender belfry
#

I have nothing else related to snippets in my settings

bitter sparrow
#

what rust-analyzer extension version? i'm on v0.3.2062

slender belfry
#

Update: the description does not get updated

#

the heck

bitter sparrow
#

even when closing and reopening vs code completely?

slender belfry
#

yep

bitter sparrow
#

weird 🥴

slender belfry
#

Where the heck is this being cached????

#

Yeah, let's stay clear of the rust-analyzer snippets

#

Seems haunted

bitter sparrow
#

another thing to check, if you use plugin snippet lower in the file, will it put the use right above the fn plugin?

#

or at the top of the file

slender belfry
#

Looks like it's a regular old VS Code snippet then

#

that does not appear in my settings

#

great

bitter sparrow
#

yeah i tried replacing bevy::prelude::* with bevy::prelude::App

#

and it put the use at the top of my file successfully

#

actually somehow it even knew to import it from bevy::app instead of bevy::prelude

#

so ig requires specifically supports a list of items that must be in scope, and it knows how to find the path where they're originally defined

#

so no glob import

slender belfry
#

AHA! I found the cache / settings

image.png
bitter sparrow
#

ah it was in there?

slender belfry
#

there we go

#

that solves it

bitter sparrow
#

well we can still use rust-analyzer snippets, we just can't have it auto-insert a glob import

#

in this case we're not currently using any of the additional features that rust-analyzer snippets support compared to vs code snippets

#

but there might be a use case?

slender belfry
#

I'm currently so skeptical about rust-analyzer snippets after this that I'd prefer VS Code snippets

#

those at least are guaranteed to work by VS Code itself

#

We can still change them to RA snippets if needed

#

But I'll approve a PR doing either

bitter sparrow
#

okay

#

anyways we can have plain text snippets as vs code snippets and if we later want a rust-analyzer snippet

#

we can add one while keeping the old snippets as vs code snippets

#

updated pr

slender belfry
#

In the end, I'll trust your judgement on this 🙂

#

huh, you removed the descriptions?

bitter sparrow
#

yep

slender belfry
#

fair enough, I guess they don't add much to such short code

bitter sparrow
#

looks like this without description

slender belfry
#

Pretty neat

#

You're missing a terminating newline

bitter sparrow
#

my vs code is being really weird about the terminating newline