These are chat archives for Ruhrpottpatriot/GW2.NET

27th
Sep 2015
Steven Liekens
@StevenLiekens
Sep 27 2015 09:05
I just noticed that our implementation of /v2/skins does not set IconFileId or IconFileSignature
but maybe we don't need that
can we remove those properties?
we only need IconFileUrl
and we can update older /v1 code to build the FileUrl from a given FileId and FileSignature
Steven Liekens
@StevenLiekens
Sep 27 2015 09:33
hmm
Steven Liekens
@StevenLiekens
Sep 27 2015 11:46
I think that we need to promote property DefaultSkinId to the base Item class
I believe that any item can unlock a skin
not just equipment
Steven Liekens
@StevenLiekens
Sep 27 2015 13:17
@Ruhrpottpatriot I need your time
I finished cleaning up the converters
according to github: 3,786 additions and 4,632 deletions.
Steven Liekens
@StevenLiekens
Sep 27 2015 13:22
so.. 846 lines of code that are no longer needed with my improvements
I'll open a PR
after I verify that everything still works
Robert Logiewa
@Ruhrpottpatriot
Sep 27 2015 14:16
ok
Steven Liekens
@StevenLiekens
Sep 27 2015 16:02
there's something wrong with how async responses are handled
there's a lot of code that accesses task.Result instead of using await task or task.ContinueWith()
the result is that when an exception is thrown, you get an AggregateException that wraps an AggregateException that wraps the base exception
annoying
Robert Logiewa
@Ruhrpottpatriot
Sep 27 2015 16:22
is Unwrap() available. If not, we should code an extension method.
Steven Liekens
@StevenLiekens
Sep 27 2015 16:23
or we could just fix the damn code :smile:
I'm writing integration tests that show where the problems are
because it's not obvious just by looking at the code
btw the real fix is to use await
do a search for task.Result and replace with await task
and mark the methods as async
that immediately fixes it
Robert Logiewa
@Ruhrpottpatriot
Sep 27 2015 18:34
Unwrap() nearly does the same thing as await, it was intended that way :)
Steven Liekens
@StevenLiekens
Sep 27 2015 18:38
I don't follow
well I just looked it up and it looks like something different
Robert Logiewa
@Ruhrpottpatriot
Sep 27 2015 18:41
It's a side effect of the await Keyword. If you have a nested type in parralelism awaitcan unwrap those types.
Steven Liekens
@StevenLiekens
Sep 27 2015 18:52
but the code is not using await
that's the problem
I'm not trying to unwrap a task result, I'm trying to unwrap an AggregateException that is thrown from code that tries to access task.Result and is itself running inside a task
I think words aren't helpful in trying to explain it :smile:
Steven Liekens
@StevenLiekens
Sep 27 2015 19:04
that fiddle is a reproduction of what I'm seeing
if you look at the stack trace then you can see what's wrong
Steven Liekens
@StevenLiekens
Sep 27 2015 19:45
oh man
I just found a way to use the API as a data source for data driven tests
like you can have a test method with a parameter (int id)
and then for every ID in the API, run that test
Steven Liekens
@StevenLiekens
Sep 27 2015 19:55
downside is that the whole thing kinda falls apart as soon as the Discover() method stops working