Equip Item Attribute

[JamesDowney]

I'm punching a bit above my weight with this one, and I'd love feedback on how to bridge the gap to something usable here.

The things I'd like to do are:

• To use the right tag, I'm not sure <Link> is the way to go.
• In-line the link with the tile header text, or at least position better than a 12% left side padding.
• Have better font size/weight.
• Not have to manually generate an item ID within the item's file (see: PowerfulGlove.tsx line 16), and instead just pass an $item in the attribute (I tried this but kept having a litany of issues).
• Handle the specification of slots for weapons and accessories (which means appending &action=dualwield and &slot=1/2/3 to the submitted URL as appropriate, and possibly supporting those specifications by appending additional links/text to the tile).
• Not render the equip link if the item is equipped.
• If this is a desirable thing, probably duplicate it for familiars.

I think there's something decent here when I can better figure out what I need to do to move forward.

This is not meant to be a finalized PR, the Powerful Glove changes are only for demonstrative purposes.

[docrostov]

i think this is a great idea!

in order to generate the link next to the header, i think you would need to put it inside like so:

<Heading as="h3" size="sm">
          {header} <EquipLink equipItem={equipItem} />
</Heading>

or potentially even have it be inside the {header} statement; as it currently exists, this should follow the rules of tags in html, where a newline will be forced after no matter what you do. to be clear, i suspect you probably tried this, and i'm not entirely sure this works either for our use case; i am mildly concerned you will continue to have it inherit the style of the heading itself rather than allowing it to be tiny and bite-sized (as i actually like the sizing you have in the screenshot), and/or <Link> itself is generating a <p> or <span> itself that will generate a newline regardless of what we do.

personally, i think it is OK if we have to manually generate an item ID for items; it would be nice to pass $item but if that requires too much scaffolding i think it is fine to do it the way you did it in this prototype.

one small overarching concern i have is that this would mean that every item tile would need to (eventually) be checking both within the tile to see if something is equipped (as almost every tile has a haveEquipped variable) and in the actual EquipLink.tsx to figure out whether it's generating the link or not. it would also (eventually) need to pass even more information, like (to your point) the type of equip it is going to do and the slot etc. i wonder if there is a way to get the tile itself to pass that information so we are not putting calls to mafia itself within /components/ and keeping mafia calls as much as possible to within the tile itself.

this is a mild issue with QuestTile as well, as it seems like every QuestTile is calling level pretty consistently, but i'm not sure if this is just a small style issue that gives me heartburn or an actual problem. i defer to bean/phred on this one obvs, but it seems a little dangerous to have the same information called multiple times per tile both within the tile and as the tile's format is being generated.

[JamesDowney]

I think I could probably create a function to generate an equip URL based on a passed $item, call that function in each <Tile>'s EquipLink attribute, then passing the completed URL as a string to the EquipLink component. This doesn't solve the issues of dynamically creating links for items that have multiple slots though.

[docrostov]

I think I could probably create a function to generate an equip URL based on a passed $item, call that function in each <Tile>'s EquipLink attribute, then passing the completed URL as a string to the EquipLink component. This doesn't solve the issues of dynamically creating links for items that have multiple slots though.

i think this is a pretty good direction IMO. you could def have that function return undefined if there's a haveEquipped flag on where EquipLink knows not to generate [equip] if you have it equipped already, solving that conundrum entirely. (this does bring up one question -- does clicking [equip] generate a new page? if so we probably need to figure out a way to make it generate [equip] and only reload character pane rather than generating a whole new page, but that's a whole other can of worms.)

[JamesDowney]

Alright, the link doesn't look terrible, it hides when equipped. My issue is generating slot links, I'm pretty much stumped. In the current state, it fails pretty gracefully though - a weapon is always equipped to the main slot, which you see when it reloads the main pane, and an accessory will throw the usual "You may only equip 3 accessories at a time." when the main pane is reloaded.

[phulin]

one small overarching concern i have is that this would mean that every item tile would need to (eventually) be checking both within the tile to see if something is equipped (as almost every tile has a haveEquipped variable) and in the actual EquipLink.tsx to figure out whether it's generating the link or not. it would also (eventually) need to pass even more information, like (to your point) the type of equip it is going to do and the slot etc. i wonder if there is a way to get the tile itself to pass that information so we are not putting calls to mafia itself within /components/ and keeping mafia calls as much as possible to within the tile itself.

this is a mild issue with QuestTile as well, as it seems like every QuestTile is calling level pretty consistently, but i'm not sure if this is just a small style issue that gives me heartburn or an actual problem. i defer to bean/phred on this one obvs, but it seems a little dangerous to have the same information called multiple times per tile both within the tile and as the tile's format is being generated.

This is really not an issue - all those calls get consolidated into one server request. They're not computationally expensive so I'm not really worried about this.

[JamesDowney]

This should be all set to go now.