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

feat: allow to rotate items in the InventoryHud #24

Merged

Conversation

stefaniamak
Copy link
Contributor

@stefaniamak stefaniamak commented Jul 8, 2020

Associated PR: Terasology/LightAndShadow#123

Result

Quickslot item selection changes completely. The active item is now stable, while the inventory items at the quickslot rotate within it in a ring-like style.

las-inventory

Changes

The main head behind the changes is @skaldarnar, who gave me the steps on how to implement this method.

  • Added the boolean variable rotateItems to set which quickslot version you want
  • Added the class TargetSlotBinding which provides the above result

If you want the correct format and order of the inventory items, please run the test alongside Terasology/LightAndShadow#123.

Shortcut file

Not sure this file was created automatically, or by mistake from me. I committed it in case it was automatically created, so to prevent failed tests.

How to test

  • Start a game within Terasology (both new or old save is fine)
  • Fill up your quickslot with at least 2 items
  • Scroll up and down

Test UI screenshot

Left image: selected item A at the bottom
Right image: quickslot after scrolling once downwards; previously selected item A went at the top, and the selected item became the one which was above the previously selected item A.

Note: Below format is done by Terasology/LightAndShadow#123. If you run tests without that PR, the selected inventory item would be the top inventory item from quickslot, and the scrolling rotation will be inverted.

inventory1 inventory2

Copy link
Member

@jdrueckert jdrueckert left a comment

Choose a reason for hiding this comment

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

Still need to test in-game.

SelectedInventorySlotComponent component =
localPlayer.getCharacterEntity().getComponent(SelectedInventorySlotComponent.class);
return (component.slot + offset) % 10;
//return null;
Copy link
Member

Choose a reason for hiding this comment

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

is that still required? if not please remove

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TargetSlotBinding gives an error if it has no getter, but are you referring to the code inside the getter instead of the getter itself? If yes, I should look more into it because that code was suggested by skaldarnar, who looked into the binding.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay after looking a bit into this, I believe it is needed because one might not need the new kind of quickslot mode. I did try to remove parts I thought might be unuseful, but kept getting an error message each time. I shall tag @skaldarnar to give a second opinion.

@jdrueckert jdrueckert marked this pull request as ready for review July 15, 2020 16:43
Copy link
Contributor

@skaldarnar skaldarnar left a comment

Choose a reason for hiding this comment

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

To avoid the empty set method instead of implementing the base interface Binding you can extend the abstract class ReadOnlyBinding - which only requires to implement the get method.
The setter is just empty in that abstract class, i.e., it's exactly the same logic 😉

@skaldarnar skaldarnar added the Type: Improvement Request for or addition/enhancement of a feature label Jul 20, 2020
@skaldarnar skaldarnar changed the title 'Rotating the items' navigation for quickslot feat: allow to rotate items in the InventoryHud Jul 20, 2020
@skaldarnar skaldarnar merged commit df5397e into Terasology:develop Jul 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Improvement Request for or addition/enhancement of a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants