I've decided to take on Issue 40 as my first code contribution.
I chose to display the equipped item below the target item. If the target item happens to be equipped, then its name is updated to indicate that and the item is not repeated for compare.
The comparison will be displayed any time info is requested on an item (eg: in the shop buy screen, or in the player's inventory).
What do you think?
Technical Questions before I even consider a pull-request:
1) How do I move the word, " (Equipped)" into a translatable resource? (It is currently hard-coded in the ItemInfoActivity source file.)
2) I started with simplicity in implementation -- the iteminfo.xml file now has a second layout for the equipped item (I call it compare item) that is identical to the original. The ItemInfoActivity class has a method for filling the content of the compare item. This is a copy of the original code to fill the target item, except for the ids, thus breaking DRY.
* Is it worth creating a single fill method that takes a list of ids?
* OR is it worth changing the XML to be a collection of items (just for 2?) and thus simplifying making a single fill method?
Useful links
Source code of the game - Contribution guide - ATCS Editor - Translate the game on Weblate - Example walkthrough - Andor's Trail Directory - Join the Discord
Get the game (v0.8.9) from Google, F-Droid, our server, or itch.io
Source code of the game - Contribution guide - ATCS Editor - Translate the game on Weblate - Example walkthrough - Andor's Trail Directory - Join the Discord
Get the game (v0.8.9) from Google, F-Droid, our server, or itch.io
Issue 40 - Weapon & Armour comparison
-
- Posts: 6
- Joined: Sun Mar 22, 2015 3:16 pm
- android_version: 4.4 - Kitkat
-
- Posts: 6
- Joined: Sun Mar 22, 2015 3:16 pm
- android_version: 4.4 - Kitkat
Re: Issue 40 - Weapon & Armour comparison
Figured out how easy it is to add the (Equipped) text as translatable resource.
As for DRY, I realized that creating a static method for each property simplifies the code tremendously. Each method takes a view object (mostly TextView) and an ItemType instance (among other things if needed).
(e.g.)
I can still see a future request to compare a weapon against both equipped hands, in case the player is dual-wielding, or considering dual-wielding and wants to look at the current shield.
As for DRY, I realized that creating a static method for each property simplifies the code tremendously. Each method takes a view object (mostly TextView) and an ItemType instance (among other things if needed).
(e.g.)
Code: Select all
fillTitle(world, resources, (TextView) findViewById(R.id.iteminfo_title), itemType, (itemType == equippedType));
fillDescription((TextView) findViewById(R.id.iteminfo_description), itemType);
fillCategory((TextView) findViewById(R.id.iteminfo_category), itemType);
fillItemEffects((ItemEffectsView) findViewById(R.id.iteminfo_effects), itemType);
fillDisplayType(resources, (TextView) findViewById(R.id.iteminfo_displaytype), itemType);
if (equippedType != null && equippedType != itemType){
findViewById(R.id.compareinfo).setVisibility(View.VISIBLE);
fillTitle(world, resources, (TextView) findViewById(R.id.compareinfo_title), equippedType, true);
fillDescription((TextView) findViewById(R.id.compareinfo_description), equippedType);
fillCategory((TextView) findViewById(R.id.compareinfo_category), equippedType);
fillItemEffects((ItemEffectsView) findViewById(R.id.compareinfo_effects), equippedType);
fillDisplayType(resources, (TextView) findViewById(R.id.compareinfo_displaytype), equippedType);
} else {
findViewById(R.id.compareinfo).setVisibility(View.GONE);
}
-
- VIP
- Posts: 573
- Joined: Sat Jun 21, 2014 4:00 am
- android_version: 9.0 - Pie
- Location: lodar's hideout
Re: Issue 40 - Weapon & Armour comparison
it will hugely improve the UI. and make it item specific i.e., if one is buying a weapon he should be able to compare b/w these and if he is buying a shield then he must be comparing with the equipped shield and same for headgears, armors, shoes and gloves. just suggesting .
- rijackson741
- Posts: 4451
- Joined: Tue Aug 20, 2013 2:04 am
- android_version: 10 - Android 10
- Location: Somewhere in Dhayavar
- Contact:
Re: Issue 40 - Weapon & Armour comparison
Don't forget that when dual wielding there can be two equipped weapons.
Level:71, XP:6493739, PV:608, FQ:84
HP:210, AC:212, AD:58-77, AP:4, ECC:16%, CM:1.5, BC:188, DR:3
Gold: 237559 | RoLS:1, RoL:1, GoW:1, VSH:1, RoFLS:1, WoB:1
HH:1, WA:1, CS:2, Cl:1, IF:4, Ev:3, Re:2, WP:DA:1, WP:1S:1, WP:B:1, AP:L:1, FS:DW:2, S:DW:1
HP:210, AC:212, AD:58-77, AP:4, ECC:16%, CM:1.5, BC:188, DR:3
Gold: 237559 | RoLS:1, RoL:1, GoW:1, VSH:1, RoFLS:1, WoB:1
HH:1, WA:1, CS:2, Cl:1, IF:4, Ev:3, Re:2, WP:DA:1, WP:1S:1, WP:B:1, AP:L:1, FS:DW:2, S:DW:1
- Zukero
- Lead Developer
- Posts: 2028
- Joined: Thu Jul 21, 2011 9:56 am
- android_version: 8.0
- Location: Eclipse
Re: Issue 40 - Weapon & Armour comparison
Sounds like a good idea.
I'll check your code soon.
However, you should base your changes on my fork : https://github.com/Zukero/andors-trail
Thanks for working on it !
I'll check your code soon.
However, you should base your changes on my fork : https://github.com/Zukero/andors-trail
Thanks for working on it !
Lvl: 78, XP: 8622632, Gold: 271542, RoLS: 1, ElyR: -, RoL: -, ChaR: 1, GoLF: 1, ShaF: 1, SRoV: 1, VSH: 1, WMC: 1, GoW: 1
HP: 71, AC: 301%, AD: 38-47, AP: 3, ECC: 50%, CM: 3.75, BC: 101%, DR: 2
HP: 71, AC: 301%, AD: 38-47, AP: 3, ECC: 50%, CM: 3.75, BC: 101%, DR: 2
-
- Posts: 6
- Joined: Sun Mar 22, 2015 3:16 pm
- android_version: 4.4 - Kitkat
Re: Issue 40 - Weapon & Armour comparison
As I've written it:
* It will show the corresponding equipped item -- eg helmet to helmet, boots to boots, etc.
* If you've dual-wielded and you look a shield, it will compare to the weapon in the off-hand.
* If you look at the weapon in your shield hand, it will compare to the weapon in your weapon hand.
All of this is a function of using the item code already in place.
For dual wielding scenarios, it might be useful to show what is equipped in both hands whenever looking at a weapon or shield. As far as I can tell, that will require a little bit of hackery in the code, since everything currently works from item definitions - that is no checks are made concerning what type of item you are looking at.
@Zukero: I already created a pull request on oskarwiksten's fork. Should I get your fork instead?
* It will show the corresponding equipped item -- eg helmet to helmet, boots to boots, etc.
* If you've dual-wielded and you look a shield, it will compare to the weapon in the off-hand.
* If you look at the weapon in your shield hand, it will compare to the weapon in your weapon hand.
All of this is a function of using the item code already in place.
For dual wielding scenarios, it might be useful to show what is equipped in both hands whenever looking at a weapon or shield. As far as I can tell, that will require a little bit of hackery in the code, since everything currently works from item definitions - that is no checks are made concerning what type of item you are looking at.
@Zukero: I already created a pull request on oskarwiksten's fork. Should I get your fork instead?
- Zukero
- Lead Developer
- Posts: 2028
- Joined: Thu Jul 21, 2011 9:56 am
- android_version: 8.0
- Location: Eclipse
Re: Issue 40 - Weapon & Armour comparison
Griff, oskar's github is deprecated, and quite a few commits behind.
Rebasing your changes on my latest "master" branch should be no problem ,as the files you changed haven't moved that much. You could then open your pr against my repo, as it is the reference currently.
Rebasing your changes on my latest "master" branch should be no problem ,as the files you changed haven't moved that much. You could then open your pr against my repo, as it is the reference currently.
Lvl: 78, XP: 8622632, Gold: 271542, RoLS: 1, ElyR: -, RoL: -, ChaR: 1, GoLF: 1, ShaF: 1, SRoV: 1, VSH: 1, WMC: 1, GoW: 1
HP: 71, AC: 301%, AD: 38-47, AP: 3, ECC: 50%, CM: 3.75, BC: 101%, DR: 2
HP: 71, AC: 301%, AD: 38-47, AP: 3, ECC: 50%, CM: 3.75, BC: 101%, DR: 2
-
- Posts: 6
- Joined: Sun Mar 22, 2015 3:16 pm
- android_version: 4.4 - Kitkat
-
- VIP
- Posts: 559
- Joined: Sun Nov 25, 2012 8:10 pm
- android_version: 6.0 - Marshmallow
- Location: Minneapolis, MN
Re: Issue 40 - Weapon & Armour comparison
This will be a nice improvement to the user interface (UI), so for that I say thank you.
Lvl78 XP9403007 Gold 248643 AP3 HP139 AC350 AD42-59 BC97 DR1
SP:D MC3 BC CS2 QL4 IF MF EB DW2
Rols1Rol2Elyr1Char1Golf1Shaf0Srov1Vsh1
Ozzy
lvl:47 HP114 AC254 AD27-37 BC112
SP:WA HH CS(2) CL CE IF(2) Reg
Rols1Rol0ElyR0Char2Shaf1Golf1Srov1Vsh1
SP:D MC3 BC CS2 QL4 IF MF EB DW2
Rols1Rol2Elyr1Char1Golf1Shaf0Srov1Vsh1
Ozzy
lvl:47 HP114 AC254 AD27-37 BC112
SP:WA HH CS(2) CL CE IF(2) Reg
Rols1Rol0ElyR0Char2Shaf1Golf1Srov1Vsh1
-
- VIP
- Posts: 231
- Joined: Tue Jul 31, 2012 12:12 am
- android_version: 2.1 - Eclair
Re: Issue 40 - Weapon & Armour comparison
That looks great. Just one question:
What happens, if both items have a lot of attributes and the box is larger than the screen?
Or when you play in landscape mode? Is the text (or the box) scrollable?
What happens, if both items have a lot of attributes and the box is larger than the screen?
Or when you play in landscape mode? Is the text (or the box) scrollable?