Skip to content

Adds backpacks! #114

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Adds backpacks! #114

wants to merge 8 commits into from

Conversation

benbot
Copy link
Contributor

@benbot benbot commented Apr 15, 2025

  • Tested that that the server runs
  • Tested that everything still works

This PR adds backpacks!

These are like inventories where users can keep collections of maps, avatars, and props made by other users.

The intent is to use these collections of items to feed a recommendation algorithm.

I also think it's a cool idea for players to have "starter packs" of props/maps/avatars/etc. that they can share around. This sets the foundation for that.

@benbot benbot requested a review from fire April 15, 2025 17:50
@benbot benbot force-pushed the benbot/backpack-inventory branch 2 times, most recently from 89983a0 to 49bac7e Compare April 15, 2025 17:53
@benbot benbot requested a review from lyuma April 15, 2025 17:53
@benbot benbot force-pushed the benbot/backpack-inventory branch from 49bac7e to 2aeb192 Compare April 15, 2025 17:56
@fire
Copy link
Member

fire commented Apr 15, 2025

Is there functionality to add, duplicate and remove backpacks? Add backpacks to backpacks? What about roles and access to the backpacks?

Edited:

Ok to be in the next pr.

@benbot
Copy link
Contributor Author

benbot commented Apr 15, 2025

Yeah, pretty much all of the features need to be added in, but the data model is weird enough that i thought getting a PR open for it first would be good.

I'm adding some very simple crud views for this, but as discussed, I'm going to try avoiding next and just use regular ol' templates

@benbot benbot force-pushed the benbot/backpack-inventory branch from 3588a3d to f9605a6 Compare April 15, 2025 18:45
@fire
Copy link
Member

fire commented Apr 15, 2025

Pretty please some unit tests :3

@dragonhunt02
Copy link
Contributor

dragonhunt02 commented Apr 15, 2025

I think we do need a gated API endpoint in backend to properly use this feature

@benbot benbot force-pushed the benbot/backpack-inventory branch from f9605a6 to 3e0a920 Compare April 15, 2025 19:05
@benbot
Copy link
Contributor Author

benbot commented Apr 15, 2025

I should clarify, this just adds the schemas and a migration.

No new features were added.

Because of the way user content works (via some macros) it was a little tricky to set up the different relationships needed.

I think it'd be good to get an OK on this data model before I try building anything else on top of it, so that's really what I'd like some feedback on here.

We can hold off on merging this until I have something built on top if ya'll want, but i'd still like some feedback on this

@benbot
Copy link
Contributor Author

benbot commented Apr 15, 2025

I'm adding some simple crud views.

When those are finished up it'll be easier to test.

@fire
Copy link
Member

fire commented Apr 15, 2025

With the recommendation algorithm from https://librecommender.readthedocs.io/en/latest/serving_guide/rust.html as long as we have user_id, item_id and label (0 or 1 for ranking). We're good.

In the future we can add jsonld attributes on either user or item but it's later.

For the future my ideal algorithm is https://librecommender.readthedocs.io/en/latest/api/algorithms/din.html.

I'm undecided if we want our labels to be 5 stars or merely is binary of a bag or not.

@dragonhunt02
Copy link
Contributor

dragonhunt02 commented Apr 15, 2025

Because of the way user content works (via some macros) it was a little tricky to set up the different relationships needed.

About it, I think it was done this way to simulate inheritance in elixir

I think it'd be good to get an OK on this data model before I try building anything else on top of it, so that's really what I'd like some feedback on here.
We can hold off on merging this until I have something built on top if ya'll want, but i'd still like some feedback on this

I see 👍

@benbot
Copy link
Contributor Author

benbot commented Apr 15, 2025

About it, I think it was done this way to simulate inheritance in elixir

yeah for sure. I think using embedded_schemas would be the idiomatic way to have something like inheritence, nowadays.

... also now that I think about it, maybe I can just have one big join table for all the different kinds of user content, instead of making a join table per user content table.

@fire
Copy link
Member

fire commented Apr 15, 2025

whats missing in this pr?

@benbot
Copy link
Contributor Author

benbot commented Apr 15, 2025

  • Some tests to ensure that the relations are set up how i expect
  • See if i can have one join table for all user_content, instead of a table per user_content type

@fire
Copy link
Member

fire commented Apr 15, 2025

Will we have more content types?

@benbot
Copy link
Contributor Author

benbot commented Apr 15, 2025

@dragonhunt02 feel free to leave a review too btw. I would've added you as a reviewer as well, but you weren't showing up in the list 😅

@benbot
Copy link
Contributor Author

benbot commented Apr 15, 2025

Will we have more content types?

I can only assume we would.

@benbot benbot force-pushed the benbot/backpack-inventory branch 2 times, most recently from e694f9e to f920ac6 Compare April 16, 2025 21:24
@benbot benbot force-pushed the benbot/backpack-inventory branch from f920ac6 to 6ff1f3d Compare April 16, 2025 23:45
@benbot benbot force-pushed the benbot/backpack-inventory branch from 6ff1f3d to da5c5dc Compare April 17, 2025 13:13
@benbot benbot requested a review from dragonhunt02 April 17, 2025 14:18
@fire fire mentioned this pull request Apr 19, 2025
6 tasks
@fire
Copy link
Member

fire commented Apr 28, 2025

Is this ready to merge?

@benbot
Copy link
Contributor Author

benbot commented May 1, 2025

@fire its done from my end, unless anyone has comments

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.

3 participants