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

The text of items with no value is incomplete in the inventory #176

Merged
merged 3 commits into from Apr 20, 2021
Merged

Conversation

szapp
Copy link
Collaborator

@szapp szapp commented Apr 18, 2021

Describe the bug
There are items that have no value. In the inventory it shows "Value:" but not number.

Expected behavior
Items with no value no longer show an empty "Value:" in the inventory text.

Additional context
The items have a value of 0 and therefor the value isn't displayed.

The bug has been reported in #177 #178 #179 #180.

@AmProsius AmProsius added the validation: required label Mar 5, 2021
@AmProsius
Copy link
Owner Author

@AmProsius AmProsius commented Mar 5, 2021

TEXT[0] = "Opens Gomez' chests.";
////COUNT[0] = ;
TEXT[1] = "Opens the store in the Ore Barons' cellar.";
////COUNT[1] = ;
//TEXT[2] = "";
//COUNT[2] = ;
//TEXT[3] = "";
//COUNT[3] = ;
//TEXT[4] = "";
////COUNT[4] = ;
TEXT[5] = NAME_Value;
COUNT[5] = value;

changed to

    TEXT[0]             = "Opens Gomez' chests.";
    ////COUNT[0]            = ;
    TEXT[1]             = "Opens the store in the Ore Barons' cellar.";
    ////COUNT[1]            = ;
    //TEXT[2]               = "";
    //COUNT[2]          = ;
    //TEXT[3]           = "";
    //COUNT[3]          = ;
    //TEXT[4]               = "";
    ////COUNT[4]            = ;
    //TEXT[5]             = NAME_Value;
    //COUNT[5]            = value;

@AmProsius AmProsius added the provided fix label Mar 5, 2021
@szapp
Copy link
Collaborator

@szapp szapp commented Mar 19, 2021

For this and all related fixes #177 #178 #179 #180 it might be possible to fix the inventory display in general:

  • Either do not show the word "value" if the value is 0,
  • Or in fact do show the value even if it is 0.

@AmProsius
Copy link
Owner Author

@AmProsius AmProsius commented Mar 20, 2021

  • Either do not show the word "value" if the value is 0,
  • Or in fact do show the value even if it is 0.

I'd go with hiding the word value to be consistent with other items (keys) that don't have a value.

@szapp
Copy link
Collaborator

@szapp szapp commented Mar 20, 2021

I'd go with hiding the word value to be consistent with other items (keys) that don't have a value.

Are there keys that don’t show the word “value”?

@AmProsius
Copy link
Owner Author

@AmProsius AmProsius commented Mar 20, 2021

Are there keys that don’t show the word “value”?

Several. For example the Rice Lord's Key:

INSTANCE ItKey_RB_01(C_Item)
{
name = "Rice Lord's Bowl";
mainflag = ITEM_KAT_NONE;
flags = 0;
value = 0;
visual = "ItKe_Key_01.3ds";
material = MAT_METAL;
description = name;
TEXT[0] = "Opens Rice Lord's chest.";
};

@szapp szapp self-assigned this Apr 5, 2021
@szapp szapp added compatibility: difficult impl: modify engine func type: session fix labels Apr 5, 2021
@szapp szapp added this to Other in Fix templates Apr 5, 2021
@szapp szapp changed the title Gomez' Key has empty value The text of items with no value is incomplete in the inventory Apr 18, 2021
@szapp szapp mentioned this pull request Apr 18, 2021
@szapp szapp mentioned this pull request Apr 18, 2021
@szapp szapp marked this pull request as draft Apr 18, 2021
@szapp
Copy link
Collaborator

@szapp szapp commented Apr 18, 2021

The fix is implemented to remove the display of "Value:" (which is determined from the string constant NAME_Value) from the inventory display if the item value is zero. As of now, the implementation is sub-optimal, because it is a script hook. That is not good in this case, because the hook will be executed 5 times per frame when the inventory is open. I will rewrite it in machine code.

@szapp szapp added this to In Progress in v1.1.0 via automation Apr 18, 2021
@szapp szapp added this to the v1.1.0 milestone Apr 18, 2021
@szapp szapp removed their assignment Apr 18, 2021
@szapp szapp requested a review from AmProsius Apr 18, 2021
@szapp szapp marked this pull request as ready for review Apr 18, 2021
@AmProsius AmProsius merged commit 38b719d into master Apr 20, 2021
v1.1.0 automation moved this from In Progress to Done Apr 20, 2021
@AmProsius AmProsius deleted the bug176 branch Apr 20, 2021
szapp added a commit that referenced this issue Apr 20, 2021
@szapp szapp moved this from Other to Engine mechanics in Fix templates Apr 26, 2021
@szapp szapp removed the validation: required label May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compatibility: difficult impl: modify engine func provided fix type: session fix
Projects
Fix templates
Modify engine function
v1.1.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants