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

Wrong icon for Mini Revenant Rytlock #601

Closed
Archomeda opened this issue Feb 12, 2018 · 6 comments
Labels

Comments

@Archomeda
Copy link
Contributor

@Archomeda Archomeda commented Feb 12, 2018

@Medyro Medyro mentioned this issue Feb 24, 2018
2 of 4 tasks complete
@Aonwy

This comment has been minimized.

Copy link

@Aonwy Aonwy commented Mar 6, 2018

The linked chat mentions that there is a different item ID actually contained in-game. Does that fix this issue, or is there additional work needed?

@Aonwy Aonwy added Needs Info Triaged and removed Triaged labels Mar 6, 2018
@Medyro

This comment has been minimized.

Copy link

@Medyro Medyro commented Mar 7, 2018

I'd say that there is additional work needed; as /v2/items still seems to return two Rytlocks (See https://api.guildwars2.com/v2/items?ids=63871,68688 ) one being the presumably right one and the other one being a presumable Test Rytlock; indistinguishable to the presumable normal one.

I would have expected there to be only one Rytlock in /v2/items and simply find myself a bit confused about why there would be two, as one would seem sufficient.

And if the presence of the two Rytlocks should be justified in some way, witch i can't really know, i would have at least expected both Rytlocks to link to different or even the same "valid" Mini Pet(s), and not ones with unexpected Icons. ( https://api.guildwars2.com/v2/minis?ids=265,319 )

I'd appreciate it if either the Icons would be adjusted, only one Rytlock would be shown in /v2/items (and /v2/minis respectively) or if there would be a way to distinguish if an Item in /v2/items is an actuall Item from within the Game or if it is only a Test item that was auto whitelisted, as i so far assumed that only Items from within the Game would make it to the API and not Test Items. You may correct me if i'm wrong with that trough; as i just can't really tell this trough the API Data only.

@Archomeda

This comment has been minimized.

Copy link
Contributor Author

@Archomeda Archomeda commented Mar 8, 2018

I made this issue on behalf of @CuriousCharr before I realized it was a test item that isn't acquirable. I don't think it's much of an issue that this item has the wrong data, more that the test items show up on the API. I think there are some other test items showing up, but I don't have the list on hand.

@Aonwy

This comment has been minimized.

Copy link

@Aonwy Aonwy commented Mar 12, 2018

Unfortunately, this is a relic of how we are handling whitelisting at this time. If it truly is an issue, we can work on blacklisting these items but that will take away time we can use to address some of the other things happening with the API. If no one objects, I'd like to close this out.

@Aonwy Aonwy removed the Needs Info label Mar 12, 2018
@Medyro

This comment has been minimized.

Copy link

@Medyro Medyro commented Mar 12, 2018

If no one objects, I'd like to close this out

No Objection from me.

that will take away time we can use to address some of the other things happening with the API

Then feel free to address some of these other things as...

If it truly is an issue

...this - altough it truly is an issue, as it is unintended behaviour of the API exposing Test Data; witch it obviously should't do - is not imminent at all (to me at the verry least) as it was just something i found out about while watching /v2/items for new Items and it's nothing i am/would really using/use anywhere else either.

Just be aware that those Items, or rather the relic (auto?) whitelisting, should need to be removed or reworked sooner or later tough in order to prevent possible future content leaks trough the API.
Also, if you all should agree on a rework of the (auto?) whitelising process, it would be great if someone could maybe then open an Issue for us to see that you have done so and also for tracking, if you wouldn't mind; otherwise i guess i could open one in a month - adressing the whitelisting issues with testitems - myself, if i should't see any reaction from your side addressing this direction.

@apoch

This comment has been minimized.

Copy link
Contributor

@apoch apoch commented Mar 12, 2018

We will revisit the way whitelisting is done at some point, because you're absolutely right that the automatic whitelist can expose too much.

So basically whitelisting needs a better solution but we will have to discuss internally how to solve it. Once we have a concrete plan I will be happy to keep the API dev community up to date. This also raises an interesting question of how to raise visibility on fixes or changes that are not originally reported via GitHub; I may float that in the Gitter chat to see what everyone thinks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.