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

Spawn items support #120

Merged
merged 5 commits into from
Jul 24, 2022
Merged

Spawn items support #120

merged 5 commits into from
Jul 24, 2022

Conversation

64kramsystem
Copy link
Member

@64kramsystem 64kramsystem commented Jul 22, 2022

Add spawn items support:

  • metadata types, and metadata loading
  • (level) loading and instantiation

The level loading (main.rs#load_items) follows the pattern of loading items metadata, then separately instantiating them.

I've removed the Pickable component, since until now, it serves no purpose. It may be reinstantiated.

I've also done some refactoring in the assets.rs module, consistently using the convenience loading API that was used only a few times.

This PR addresses the first entry in #83.

@64kramsystem 64kramsystem marked this pull request as draft July 22, 2022 23:50
@64kramsystem
Copy link
Member Author

@zicklag Do you know why the AssetLoader is not invoked at all in this branch (here)?

I've put everything in place, but assets under /items/ are ignored. I've tried to figure out what triggers a load, but looking at fighters for example, doesn't seems to lead to any explicit loading code (so I take there's an implicit load somewhere).

@zicklag
Copy link
Member

zicklag commented Jul 22, 2022

You need to load them as a part of the level load with another block like this one:

https://github.com/64kramsystem/punchy-dev/blob/ae35d77424e3885e8ddd045f7691335224738ecf/src/assets.rs#L169-L177

@64kramsystem
Copy link
Member Author

Ah! I need to do all the manual load 😳 I'll try this, thanks 😄

@zicklag
Copy link
Member

zicklag commented Jul 23, 2022

Thanks! I'm actually already doing that here.

No, you have to do it in the Level asset loader. The level asset loader is responsible for telling the asset server that it depends on the item assets ( which is why your item asset loader wasn't getting run ), and for setting the item handles to their actual handles in the level metadata, otherwise they will default to null handles.

Edit:

Ah! I need to do all the manual load flushed I'll try this, thanks smile

Yep. :)

@64kramsystem 64kramsystem self-assigned this Jul 23, 2022
@64kramsystem 64kramsystem marked this pull request as ready for review July 23, 2022 17:46
@64kramsystem 64kramsystem requested review from zicklag and odecay and removed request for zicklag July 23, 2022 17:49
@64kramsystem 64kramsystem changed the title Spawn items Spawn items support Jul 23, 2022
@64kramsystem
Copy link
Member Author

@erlend-sh CC

@64kramsystem 64kramsystem requested a review from zicklag July 23, 2022 20:04
@64kramsystem 64kramsystem mentioned this pull request Jul 23, 2022
Copy link
Member

@zicklag zicklag left a comment

Choose a reason for hiding this comment

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

Looks great!

I like your attention to code style and quality and improvement. 👍

@zicklag zicklag merged commit cb43a82 into fishfolk:master Jul 24, 2022
@64kramsystem 64kramsystem deleted the spawn_items branch July 24, 2022 09:28
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