kevinclement/SimpleArmory

Class Hall Titles uncompletable

Opened this issue · 3 comments

Briefly explained this in my latest PR, but might've not been a clear explanation;

image
image

As you can see I have all class hall mounts, which come after the titles, so I should have all titles marked as unlocked as well. I think this is because they are marked as quests and the quests are checked on a per-character basis, and a single character can only be one class and therefore never complete all of them.

@kevinclement you suggested I show the issue on a branch in my repo but I'm not sure how I would go about that, as it's more with the display as a whole. What I tried to do is bring back the 'allowableClasses' attribute you removed in #585, however that didn't seem to do anything - Maybe there was another bit of code to it that was removed in another commit?

ah, gotcha, I totally misread what you were asking and when you said 100% I thought it was a question about the layout of the page and not about completion percentage. I understand now.

I went and had a look at what is getting returned from the API and, no surprise it doesn't return those titles even if you've earned them. I also went and checked to see if maybe we could key off the quest being complete, but I checked and the quest doesn't show as completed for the character either, I've seen them do that for some quests.

Out of curiosity, can you actually assign any of those cross class titles on your main? Does the game let you do it? I haven't yet done this grind on my alts.

I see two ways we could "solve" it.

  1. If we want them to show up, but be properly "earned", we could hack in a special one off check where we look if they have the mount and if so, then award the title.
  2. Alternatively, we could introduce allowable classes into the titles, like we had in mounts, I think thats what you were getting at. You'd need to modify the titles json to introduce that field, and looking at the parsing logic I didn't remove the logic that checks for that field. If this is what you did, then might be a subtle bug somewhere I'd need to just step through it.

The choice of which direction I guess depends on if you want those titles to be there and filled in. I'd probably lean that direction, but would let the game be the tie breaker. If the game lets you select that title, then we should for sure be showing it and we'd need to hack in the mount check.

Out of curiosity, can you actually assign any of those cross class titles on your main? Does the game let you do it? I haven't yet done this grind on my alts.

No, they are class exclusive, just like the class hall mounts - but since we show those (and they count to the account total) figured we should be consistent with this.

2. Alternatively, we could introduce allowable classes into the titles, like we had in mounts, I think thats what you were getting at. You'd need to modify the titles json to introduce that field, and looking at the parsing logic I didn't remove the logic that checks for that field. If this is what you did, then might be a subtle bug somewhere I'd need to just step through it.

Yeah this is what I tried, also in my opinion the best option to stay consistent, worst case I'd say we just check for the mount.
Could you have a look at some point to see what may potentially be causing the allowableClasses attribute being ignored for titles?

Ya, I can have a look hopefully in next couple days. Need to make sure I submit my taxes at some point too :)