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

Feature/inventory system #69

Merged
merged 157 commits into from
Dec 18, 2023

Conversation

cayb0rg
Copy link
Contributor

@cayb0rg cayb0rg commented Aug 17, 2022

Adventure Inventory System

Creators can create their own items and select or upload icons to these items.

Creators can select items to give players upon visiting a node.

Creators can require certain items for answer choices to be selectable.

The widget will let the player know they are missing an item.

Players receive an alert when they receive an item and can view their inventory by clicking on the backpack icon.

Other changes in this PR include:

  • Node editor windows are made more responsive and roomier
  • Player converted to flex containers
  • Short answer string matching now has the option to be case sensitive and/or special character sensitive
  • Updates to MWDK3

What's Left

  • Player Accessibility
    • Hide background and other options when modals are open
    • Add aria-labels to buttons and other inventory items
    • Add aria-live updates for new and removed inventory items
  • Ensure scoring works with MWDK3
  • Update creator's and player's guides to match latest style changes

@clpetersonucf
Copy link
Member

It'll take me a little bit to run through everything, but I wanted to start working on feedback items asap. A few suggestions so far:

  • The required items UI (for multiple choice and hotspot) and the "give player items" options in the creator can probably be hidden when the inventory is empty, as a way to visually "opt out" of the feature if the user chooses not to interact with it.
  • Similarly, the "Gives item(s)" mouseover tooltip when hovering over a node shouldn't be visible unless it actually gives an item.
  • I like the Add Item UI in the creator, but it's not immediately obvious that I can further edit the item once I create it. Could the edit item dialog items become visible by default once you enter a name?
  • The Required Items UI will need a polish pass or two, but I'm not sure how to improve it without mocking up a few ideas. I'll follow up on this one.
  • Short Answer nodes in the creator don't appear to have the Required Items UI, but I think I agree that the combination of its existing requirements plus required items may be too complicated. This isn't really a feedback item, just acknowledging the decision to not include Require Items for Short Answer nodes.
  • Icons are not provided with new items by default, but the player will crash if an icon is not present.
  • The answer choice UI for multiple choice nodes in the creator does not appear to scroll far enough and causes occlusion issues with > 1 answer choices.
  • The flex enhancements to node edit screens in the creator are terrific. It's clear we're still dealing with some significant vertical content compression issues though. I wonder if we can add another responsive option, for wide enough window width, that splits the screen in half with question/asset content management on the left and answer content management on the right (for MC and SA node types)?

@cayb0rg
Copy link
Contributor Author

cayb0rg commented Aug 18, 2022

Thank you so much for the feedback! For the Required Items UI, perhaps a simple "Require Items" button next to Choose Destination that opens a modal would suffice (and would appear only if there are items). This could also make it simpler to include with the Short Answers.

Copy link
Contributor

@FrenjaminBanklin FrenjaminBanklin left a comment

Choose a reason for hiding this comment

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

There's a lot of potential in the new additions.

I think the working area in the creator can get a bit cramped - some elements can potentially be scaled down, font sizes shrank, etc. to make a little bit more space, but functionally everything new seems to be working well.

Requiring the usage of the 'advanced options' to specify both a minimum and maximum of zero for any given item instead of just allowing the single 0 feels a bit awkward - Potentially treat a single zero as 'the user must have none of this item' and have an optional toggle for 'the user can have any number of this item, even zero' - though that'd be basically the same as the path just being available at all times.

Otherwise, barring some additional things being worked on in separate PRs, this all looks good to me.

Copy link
Member

@clpetersonucf clpetersonucf left a comment

Choose a reason for hiding this comment

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

With a feature this large, there's likely still going to be additional items to address, but we've tested this pretty well and it feels like a really solid foundation to the inventory system. Approved.

@clpetersonucf clpetersonucf merged commit c90e00b into ucfopen:master Dec 18, 2023
1 check passed
@cayb0rg cayb0rg deleted the feature/inventory-system branch January 3, 2024 17:17
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