ruhrpottpatriot/gw2.net

A user friendly wrapper around the official GW2 API

gw2.net's Issues

Unable to decode some chat links

The chat link decoder uses the BinaryReader class to convert chat links from a byte array. The code calls BinaryReader.PeekChar() to check whether the end of the byte array is reached. This call fails with an exception when the next byte or bytes cannot be converted to a unicode character.

while (reader.PeekChar() != -1)
{
    ...
}

The code must be changed to never convert bytes to characters.

Changing Collection properties to conform to CA1819 and CA2227

As of now our collection properties violate the CA1819 and CA2227 rules for managed code.
We should change the collection properties to conform to those rules, as seem exemplary below. This is the recommended way to do things and should not affect performance in any significant way.

public ICollection<CombatAttribute> Attributes { get; private set; } = new Collection<CombatAttribute>(0);

public void SetCollection(IEnumerable<CombatAttribute> attributes)
{
    if(attributes == null)
    {
        throw new ArgumentNullException();
    }
    this.Attributes = attributes;
}

Since the C# compiler generates instance methods in the MSIL for properties, we can remove the setter and replace it with a method which just assigns the collection. This shold not degrade performance

References:
#30

Make collection properties not-nullable

Entities that have a collection property should initialize that property to an instance of an empty array. Property setters should throw when the given value is null. This is to avoid null reference exceptions when iterating over a collection property.

Example in class InfixUpgrade

private static readonly CombatAttribute[] Empty = new CombatAttribute[0];
private ICollection<CombatAttribute> attributes = Empty;

public ICollection<CombatAttribute> Attributes
{
    get { return this.attributes; }
    set
    {
        if (value == null)
            throw new ArgumentNullException();
        this.attributes = value;
    }
}

The Empty array must be cached to avoid multiple object allocations.

Too much equality?

Many entity classes implement interfaces like IEquatable and IComparable, but their implementation is questionable. What makes two API results equal? Is it their object ID in the API? Is it their localized name?

As a user, I would expect that reference equality is the only absolute indicator. Every other form of comparison would be implementation-specific (custom grouping and sorting etc.), and does not belong in the core library, but rather in a custom class that implements IComparer<TEntity>.

Should the current implementations be left as-is, changed or removed entirely?

https://msdn.microsoft.com/en-us/library/vstudio/7h9bszxx(v=vs.110).aspx
http://www.informit.com/articles/article.aspx?p=2425867

Unable to retrieve backpack skins

The code has a precondition that backpack skins always have a details object, but backpack skins never have one. It's not possible right now to retrieve backpack skins.

Localizable strings

[Moved from Codeplex: https://gw2dotnet.codeplex.com/workitem/1239]

Right now, it is still possible to pass in a CultureInfo object per request. I'm thinking about switching to Thread.CurrentUICulture for /v2 services. You'd still be able to pass your own CultureInfo object into the constructor. When you don't, the current UI culture will be used instead.

Info: http://msdn.microsoft.com/en-us/library/system.threading.thread.currentuiculture(v=vs.100).aspx

Ruhrpottpatriot wrote Sep 22, 2014 at 4:31 AM

Yup, do it. Seems like a good default.

StevenLiekens wrote Oct 19, 2014 at 6:02 AM

I settled on a slightly different approach. I did not like the ability to change the language once a service object has been created. If a user wants to retrieve details in 4 languages, then the user should create 4 different instances of the endpoint class.

In the end, I decided on factory classes that configure endpoints for a given language. The endpoint object itself is immutable, so the language cannot be changed afterwards.

Example: ItemFactory for /v2/items

This factory class has an indexed property that expects a two-letter language code. It uses that index value to configure the item service object.

public IRepository this[string language]
{ 
   get
   {
      return new ItemService(this.ServiceClient) { Culture = new CultureInfo(language) };
   }
} 

The code wouldn't be complete without a default option though.

public IRepository Default
{ 
   get { return new ItemService(this.ServiceClient); }
} 

Usage

// Explicit language
var itemsDE = GW2.V2.Items["de"];
var itemsEN = GW2.V2.Items["en"];
var itemsES = GW2.V2.Items["es"];
var itemsFR = GW2.V2.Items["fr"];

// Default language (API decides what language to use)
var items = GW2.V2.Items.Default; 

System language
So what about automatically determining the preferred language? I was thinking about adding helpers to the factory class.

public IRepository ForCurrentCulture
{
   get { return this[CultureInfo.CurrentCulture.TwoLetterISOLanguageName]; }
}

public IRepository ForCurrentUICulture
{
   get { return this[CultureInfo.CurrentUICulture.TwoLetterISOLanguageName]; }
}

Usage

// Current system language
var items = GW2.V2.Items.ForCurrentCulture; 

// Current UI language
var itemsUI = GW2.V2.Items.ForCurrentUICulture;

Any thoughts on this? I really need all the feedback I can get on what goes into the public API.

Transport Timeout Errors

[Moved from Codeplex: https://gw2dotnet.codeplex.com/workitem/1216]

When a timeout occurs, the default behavior is to throw a System.Net.WebException. Would it make sense to throw a System.TimeoutException instead? Timeouts are not as extreme as most other transport errors, and are usually caused by network congestion instead of a bad network configuration.

Ruhrpottpatriot wrote Aug 10, 2014 at 8:10 AM

I think it'd be best to throw a WebException with a TimeoutException being the inner exception

StevenLiekens wrote Aug 10, 2014 at 9:05 AM

We can't throw a new WebException and also preserve the original exception details that were generated by the framework. But we can catch the source WebException and wrap it in a new TimeoutException. So client code would look like this:

try
{
   // Talk to the service service.GetBuild();
} 
catch (ServiceException ex)
{ 
   // Handle errors generated by the server
}
catch (TimeoutException ex)
{ 
   // Handle timeouts
}
catch (WebException ex)
{
   // Handle fatal errors
}

Add property 'Item.DefaultSkinId'

Right now, only equipment item types have a DefaultSkin property:

• Armor.DefaultSkinId
• Weapon.DefaultSkinId
• Backpack.DefaultSkinId
• GatheringTool.DefaultSkinId

This is not in line with the json schema, which defines field default_skin on the root object.

I now believe that the game is not limited to what item types can have a default skin, and that any item type can be used to unlock a skin. It's easy to imagine a consumable item type that unlocks a skin but cannot itself be equipped.

Introducing an abstract class implementing IRequest and ILocalizable

[Moved from Codeplex: https://gw2dotnet.codeplex.com/workitem/1317]

Seeing that most of the time a implementation for a request looks like this:

public class ContinentRequest : IRequest, ILocalizable
{
    /// <summary>Gets or sets the locale.</summary>
    public CultureInfo Culture { get; set; }

    /// <summary>Gets the resource path.</summary>
    public string Resource
    {
        get
        {
            return "v2/continents";
        }
    }

    /// <summary>Gets the request parameters.</summary>
    /// <returns>A collection of parameters.</returns>
    public IEnumerable<KeyValuePair<string, string>> GetParameters()
    {
        // Get the 'lang' parameter
        if (this.Culture != null)
        {
            yield return new KeyValuePair<string, string>("lang", this.Culture.TwoLetterISOLanguageName);
        }
    }

    /// <summary>The get path segments.</summary>
    /// <returns>The <see cref="IEnumerable{T}"/>.</returns>
    public IEnumerable<string> GetPathSegments()
    {
        yield break;
    }
}

Wouldn't it make sense to implement an abstract class which has the following layout:

/// <summary>Represents a request, targeting any the v2/ endpoint.</summary>
public abstract class LocalizableRequest : IRequest, ILocalizable
{
    /// <summary>Gets or sets the locale.</summary>
    public CultureInfo Culture { get; set; }

    /// <summary>Gets the resource path.</summary>
    public abstract string Resource { get; }

    /// <summary>Gets the request parameters.</summary>
    /// <returns>A collection of parameters.</returns>
    public virtual IEnumerable<KeyValuePair<string, string>> GetParameters()
    {
        // Get the 'lang' parameter
        if (this.Culture != null)
        {
            yield return new KeyValuePair<string, string>("lang", this.Culture.TwoLetterISOLanguageName);
        }
    }

    /// <summary>The get path segments.</summary>
    /// <returns>The <see cref="IEnumerable{T}"/>.</returns>
    public virtual IEnumerable<string> GetPathSegments()
    {
        yield break;
    }
}

I went ahead and added this to the GW2.NET core library. However the only endpoint that uses this class is the continents endpoint which I am currently adding.

Thoughts?

Migrate to netcore 2.0

Since netcore has matured quite abit and PCL have become deprecated we should migrate our code to netcore where possible. Currently this requires a lot of handwork, but should be done as soon as possible.

To migrate a PCL project completely to netcore (i.e. netcoreapp2.0 and new .csproj file format) do the following steps:

1. Unload the project and open the *.csproj file.
2. Replace the content of the file with the following content:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>netcoreapp2.0</TargetFramework>
    <RootNamespace>GW2NET</RootNamespace><!-- Change root namespace as required -->
  </PropertyGroup>
  <ItemGroup>
    <None Remove="stylecop.json" />
  </ItemGroup>
  <ItemGroup>
    <AdditionalFiles Include="$(SolutionDir)stylecop.json" />
  </ItemGroup>
</Project>

3. Reinstall all missing NuGet packages again. Always install Stylecop.Analyzers.
4. Delete the packages.config and project.json file and AssemblyInfo.cs
5. Fix the remaining errors, so the code compiles again.

I'm currently looking into a way to add the global stylecop analyzers file suited for our needs. When I find something, I'll update this issue.

Additional Information:

• Target multiple frameworks with netcore csproj: https://stackoverflow.com/questions/42747977/how-do-you-multi-target-a-net-core-class-library-with-csproj

  ref: #34

How to iterate over pages with custom Items?

Hey,
How can I get the item names of a custom list of item ids? I can not create pages to get the item names faster, item by item takes too long. I tried to extend the example in the wiki with LINQ queries to filter the items, but unfortunately I can not do it.

So, how can I create pages with my own itemids to iterate over the pages and items?

The wrapper is very useful and I use many things of it. But i do not get solved my problem described above 🤣

Use HttpClient instead of ServiceClient

Discussed a long time ago. The move to netcore 2.0 should finally fix this.

https://github.com/richardszalay/mockhttp dropping this to remind me that there is a HttpClient test library out there. And https://github.com/dotnet/corefx/issues/1624 in general on testing HttpClient.

Proposed changes for 2.0

Currently each of our repositories implements IRepository, which in turn implements all other repository interfaces we currently have. This adds some inflexibility to endpoints which do not allow for some operations. Solutions are:

1. Leave IRepository as it is and add NotSupportedException to the repositories
2. Refactor IRepository into smaller interfaces and implement these interfaces on a need basis.

I advocate 2, not only are empty interfaces considered code smell to some degree, but our current pattern also violates the Interface segregation principle of SOLID.

In addition to refactoring out current interfaces, we should do some housekeeping. Current methods should either be be renamed to already established methods in C# (i.e. All(), Single(), etc.), or be renamed to CRUD method names (i.e. Get()).

Furthermore we also have to discuss, whether our code should fail-silent or not. Personally I advocate for a fail-fast approach. Therefore we should translate any HTTP-Statuscode into proper exceptions. As a curtesy to the user, we also could add fail-silent methods to the repositories (much akin to the ...OrDefault methods in LINQ).

Obviously every repository method is executed asynchronous and synchronous methods have been removed completely, since they serve no purpose anymore.

The current v2 repository layout looks like the code below:

public class ColorService : ServiceBase<ColorPalette>, IDiscoverService<int>, IApiService<int, ColorPalette>, ILocalizable
{
    public ColorService(
        HttpClient httpClient,
        HttpResponseConverter responseConverter,
        ICache<ColorPalette> cache,
        IConverter<IEnumerable<int>, IEnumerable<int>> identifiersConverter,
        IConverter<ColorPaletteDTO, ColorPalette> colorConverter)
        : base(httpClient, responseConverter, cache)
    {}

    public CultureInfo Culture { get; set; }

    public Task<IEnumerable<int>> DiscoverAsync() {}

    public async Task<IEnumerable<int>> DiscoverAsync(CancellationToken cancellationToken) {}

    public Task<IEnumerable<ColorPalette>> GetAsync(CancellationToken cancellationToken) {}

    public Task<IEnumerable<ColorPalette>> GetAsync(CancellationToken cancellationToken, params int[] identifiers) {}

    public async Task<IEnumerable<ColorPalette>> GetAsync(IEnumerable<int> identifiers, CancellationToken cancellationToken) {}

    public async Task<ColorPalette> GetAsync(int identifier, CancellationToken cancellationToken) {}

    public async Task<IEnumerable<ColorPalette>> GetAsync(Func<ColorPalette, bool> selector, CancellationToken cancellationToken) {}
}

public abstract class ServiceBase<T>
{
    protected ServiceBase(HttpClient client, HttpResponseConverter responseConverter, ICache<T> cache)
    {
        this.Client = client;
        this.ResponseConverter = responseConverter;
        this.Cache = cache;
    }

    public HttpClient Client { get; }

    public HttpResponseConverter ResponseConverter { get; }

    public ICache<T> Cache { get; }

    protected IEnumerable<int> CalculatePageSizes(int queryCount)
    {
        if (queryCount <= 200)
        {
            return new List<int> { queryCount };
        }

        return new List<int> { 200 }.Concat(this.CalculatePageSizes(queryCount - 200));
    }
}

There's a question that pops into my mind for repositories with localized output. Should we deal with localization by setting a property on the repository, which sets the locale for all further requests until changed, or should we add a (optional) method parameters which only set the locale for the next request.
In my opinion the latter is the more sensible solution. The cache already does not care about the locale and stores everything. When accessing it, the user can either specify a locale or not. In the former case only results matching the locale are returned, in the latter case everything matching the filter is returned, regardless of the culture.

The HttpResponseConverter is a special convert, which does not inherit from IConverter<TIn, TOut>. It's ConvertAsync method accepts a HttpResponseMessage, an inner converter, an optional state and a Cancellation token. The method then deserializes the response into the appropriate data contract, before passing the results to the inner converter, which then converts the data contracts into actual objects.

Support empty parameters

[Moved from Codeplex: https://gw2dotnet.codeplex.com/workitem/1342]

The API will soon support empty query parameters. This means that we don't have to check for that case anymore, and just let the API deal with it.

See: arenanet/api-cdi#24

TODO (up for grabs): remove all code that checks for empty parameters

NOTE: the code should still assert that collection types are not null.
For collections, the only change is that a collection argument with Count == 0 is now a valid argument.

Parsing 'Link' headers

[Moved from Codeplex: https://gw2dotnet.codeplex.com/workitem/1246]

All services that support pagination also provide metadata for the current page through 'Link' headers. The format is insanely complicated though. See RFC 5988, section 5.

GW2NET.Skins does not compile

The GW2NET.Skins project does not compile, since the DetailsDataContract and ConverterForObject classes are missing.

Regression in Paginator

edit
The RetryOnFault() addition to class Paginator introduced a threading issue in FindAllPagesAsync() that causes three errors:

• some pages are duplicated (same page is requested multiple times)
• other pages are skipped
• the page index goes out of range (last page index + 1)

Replace 'GetChatLink()' with a pregenerated string

/v2/items now has a chat_link field.

We already supported chat links in the form of Item.GetChatLink() : ItemChatLink. It would be appropriate to remove the GetChatLink() method and replace it with a ChatLink text property.

For /v1/item_details.json, the V1 item converters can be updated to generate the missing chat link.

We should probably do the same thing for recipes and POI chat links while we're at it.

Interleave tasks in FindAllPagesAsync

The FindAllPagesAsync extension method returns a collection of tasks that complete in order that they were started.

GW2.NET/src/GW2NET.Core/Common/Paginator.cs

Line 119 in 1fd0356

public static IEnumerable<Task<ICollectionPage<T>>> FindAllPagesAsync<T>(this IPaginator<T> instance, int pageSize, int pageCount, CancellationToken cancellationToken)

Instead, the tasks should be interleaved so that tasks are returned in their order of completion, using this task combinator. It is a small change that dramatically improves performance by minimizing idle CPU time between incomplete requests.

https://msdn.microsoft.com/en-us/library/hh873173(v=vs.110).aspx

static IEnumerable<Task<T>> Interleaved<T>(IEnumerable<Task<T>> tasks)
{
    var inputTasks = tasks.ToList();
    var sources = (from _ in Enumerable.Range(0, inputTasks.Count) 
                   select new TaskCompletionSource<T>()).ToList();
    int nextTaskIndex = -1;
    foreach (var inputTask in inputTasks)
    {
        inputTask.ContinueWith(completed =>
        {
            var source = sources[Interlocked.Increment(ref nextTaskIndex)];
            if (completed.IsFaulted) 
                source.TrySetException(completed.Exception.InnerExceptions);
            else if (completed.IsCanceled) 
                source.TrySetCanceled();
            else 
                source.TrySetResult(completed.Result);
        }, CancellationToken.None, 
           TaskContinuationOptions.ExecuteSynchronously, 
           TaskScheduler.Default);
    }
    return from source in sources 
           select source.Task;
}

There is also a synchronous method named FindAllPages that returns a collection of Lazy<TResult> instead of Task<TResult>. Because it is synchronous, I don't think that it can be interleaved in this way. Any method that interleaves Lazy objects would probably still use TaskCompletionSource under the hood, defeating the purpose of synchronous methods. One more reason to drop synchronous API calls.

ColorConverter does not apply ItemId properly

While coding the new repository layout, I found out that the ItemId property is not converted properly by the ColorPaletteConverter. This is a bug that affects every version and thus should be fixed in the 1.4.0 branch

Fix/improve tests

There are a good number of integration tests that enumerate the entire API like a real app would. These kind of tests are always flaky, so don't be surprised by the fact that they are failing.

The biggest reason why tests are failing right now is because the API is now throttled. Running all integration tests generates more GETs than the API "wants to" handle.

Another reason for failures is that some v1 APIs like /v1/event_names.json have been turned off completely.

SerializationException from Mumble Link

When reading from the mumble link memory-mapped file, it is possible that the game client is writing to the file at the same time the library is reading from it. As a result, a SerializationException is thrown.

What should the behavior of the library be if that occurs? Catch it and try again? Catch it and return a null value? Catch it and re-throw?

Help getting started please

Hi there,
Not really sure if this is where to post this and ask a question being a bit inexperienced but here goes...

I've just been trying to startup the project by following the documentation here and I can't get my head around some of the basics of the setup.
I am inexperienced with C# and VS, I must be overlooking something quite simple though I think.
So far I've attempted to set up the project by opening VS with the .NET desktop development, then opening a copy of the source code and opening the GW2.NET.sln file.

I have checked the references for GW2NET.dll and GW2NET.Core.dll (all there I'm pretty sure), and sure to build GW2NET.Tests and then I initialise interactive with project. Then I make sure to be using GW2NET and GW2NET.Common as the documentation says.
When I try to create the variables for the facade class I get an error,
(1,10): error CS0103: The name 'GW2' does not exist in the current context, after entering this, var v1 = GW2.V1;, whilst in the C# interactive console in VS (same for both).

Before this I had tried to attempt using the Items API, following the how to for returning a list of items (here and I get the same error about GW2 not existing in the current context.

I would love any help with this as it's something I'm very interested in learning about!

Other info:
-Running VS 2017
-.NET framework 4 - 4.6
-Windows 10 64 bit

Missing mappings 1.3

This is a list of broken Json to object mappings that I found in version 1.3.0. It is probably incomplete and will be updated as I find more missing mappings.

Note: _ means unnamed and refers to the root json object.

/v1/item_details.json

• _.consumable.unlock_type missing type "Outfit"
• _.consumable.unlock_type missing type "GliderSkin"
  • https://api.guildwars2.com/v1/item_details.json?item_id=67054
• _.consumable.unlock_type missing type "Champion"
  • https://api.guildwars2.com/v1/item_details.json?item_id=70076
• _.consumable.type missing type "TeleportToFriend"
  • https://api.guildwars2.com/v1/item_details.json?item_id=67280
• _.consumable.skins missing field
  • https://api.guildwars2.com/v1/item_details.json?item_id=8466
• _.minipet.minipet_id missing field
  • https://api.guildwars2.com/v1/item_details.json?item_id=20153
• _.weapon.infix_upgrade.attributes.attribute missing type "BoonDuration"
  • https://api.guildwars2.com/v1/item_details.json?item_id=74735
• _.weapon.infix_upgrade.attributes.attribute missing type "ConditionDuration"
  • https://api.guildwars2.com/v1/item_details.json?item_id=70918
• _.consumable.extra_recipe_ids missing field
  • https://api.guildwars2.com/v1/item_details.json?item_id=74301

/v2/items

• _.details.unlock_type missing type "Outfit"
• _.details.unlock_type missing type "GliderSkin"
  • https://api.guildwars2.com/v2/items/67054
• _.details.unlock_type missing type "Champion"
  • https://api.guildwars2.com/v2/items/70076
• _.details.type missing type "TeleportToFriend"
  • https://api.guildwars2.com/v2/items?id=67280
• _.chat_link missing field
• _.details.skins missing field
  • https://api.guildwars2.com/v2/items/8466
• _.details.minipet_id missing field
  • https://api.guildwars2.com/v2/items/20153
• _.details.infix_upgrade.attributes.attribute missing type "BoonDuration"
  • https://api.guildwars2.com/v2/items/74735
• _.details.infix_upgrade.attributes.attribute missing type "ConditionDuration"
  • https://api.guildwars2.com/v2/items/70918
• _.details.extra_recipe_ids missing field
  • https://api.guildwars2.com/v2/items/74301

/v1/recipe_details.json

• _.type missing type "LegendaryComponent"
  • https://api.guildwars2.com/v1/recipe_details?recipe_id=10000
• _.type missing type "GuildConsumableWvw"
  • https://api.guildwars2.com/v1/recipe_details?recipe_id=10184

/v2/recipes

• _.chat_link missing field
• _.type missing type "LegendaryComponent"
  • https://api.guildwars2.com/v2/recipes/10000
• _.type missing type "GuildConsumableWvw"
  • https://api.guildwars2.com/v2/recipes/10184

/v1/skin_details.json

• _.type missing type "Gathering"
  • https://api.guildwars2.com/v1/skin_details.json?skin_id=6055
  • https://api.guildwars2.com/v1/skin_details.json?skin_id=6063
  • https://api.guildwars2.com/v1/skin_details.json?skin_id=6075

/v2/skins

• _.type missing type "Gathering"
  • https://api.guildwars2.com/v2/skins?ids=6055,6063,6075

/v1/colors.json

• _.item missing field
• _.categories missing field

/v2/colors

• _.item missing field
• _.categories missing field

/v2/worlds

• _.population missing field

/v2/commerce/prices

• _.whitelisted missing field

Mumble Link

• _.profession missing mapping for value 9 (revenant)

HTTP 503 (Service Unavailable)

There is a special kind of response that is used when the API is down. Let's make sure it is handled properly.

HTTP/1.1 503 Service Unavailable
Content-Length: 30
Content-Type: application/json; charset=utf-8
Server: Quaggans
X-Rate-Limit-Limit: 600
X-Content-Type-Options: nosniff
Access-Control-Allow-Origin: *
X-Powered-By: ARR/2.5
X-Frame-Options: SAMEORIGIN
X-Content-Type-Options: nosniff
X-XSS-Protection: 1; mode=block
Strict-Transport-Security: max-age=31536000; includeSubDomains; preload
Date: Sun, 24 Sep 2017 09:10:08 GMT

{
  "text": "API not active"
}

Add missing SharedAssemblyInfo to projects

Multiple projects are missing a reference to SharedAssemblyInfo, we either should add it, or move away from it altogether.

I'm personally more for a move away, since we can't increase the version of projects independently. We should be able to do that however.

Use the 'async' and 'await' keywords

A lot of code still uses the old .NET 4.0 style of programming with Task using Task.ContinueWith() instead of the newer await keyword. In a few files, the ContinueWith pattern is implemented poorly so that when a task is faulted, its Task.Exception is wrapped in two layers of AggregateException instead of just one.

How to get a list of any Items

hello ^^ i begin with your api.
and you method FindAll() for the items is unable :/

i try to get any items page but i have an expetion on the page 137. the api say" precondition: value =! null" but when i do "if(xxxx.FindPage(137) != null )" , it's not ok :/

So , how can i get a list of any items ? :)
thank you :P

Mumble API is throwing NullReferenceException

I've recently started playing around with the Mumble API and every time I want to get the map name, it throws a NullReferenceException. I got my game running and I'm standing right in Lions Arch. Can this be caused by the "virtual" property in the wrapper? I'm relatively inexperienced as I'm learning C# by myself in my freetime, so I have no experience with virtuals.

Here is my code snippet:
string mapname = avatar.Identity.Map.MapName;

Guild Emblems

[Moved from Codeplex: https://gw2dotnet.codeplex.com/workitem/1221]

We have a way to retrieve RGB color information, but we do not have a way to retrieve guild emblem icons. I know that the wiki has a complete gallery of emblems, but it's not clear if we are able/allowed to use that.

http://wiki.guildwars2.com/wiki/Gallery_of_guild_emblems

Also relevant: https://forum-en.guildwars2.com/forum/community/api/Use-of-Guild-Emblems

What can we do about this?

Invalid chat links

Chat link converters were improved in version 1.4.0, but unfortunately the new converters use wrong header bytes for skills, traits, recipes, skins and outfits.

https://github.com/Ruhrpottpatriot/GW2.NET/blob/20adafec3d61bac8dc7d549a42e91e570d6e3f70/src/GW2NET.Core/ChatLinks/Interop/Header.cs

The Header enumeration must be changed to match the headers listed on the wiki: https://wiki.guildwars2.com/wiki/Chat_link_format#Header

Implement Dependency Injection

ref #34 #5

Update master with fixes from maintenance

The maintenance branch is currently many commits ahead of master.

PS: when merging, rename "bloodlust" to "Bloodlust"

ConverterForObject missing

Somehow the ConverterForObject<T> is missing from the repository.

Change NuGet Package information to .NET Patform Standard

A week ago MS released a new way of marking your packages as compatible with other versions. We should change our NuGet Packages accordingly.

See HERE for more information.

Increase type visibility

[Moved from Codeplex: https://gw2dotnet.codeplex.com/workitem/1343]

A lot of the classes used throughout the code are internal. This was done to keep the public API clean --> better IntelliSense. As a result, a lot of extensibility points are now blocked.

TODO
Convert the visibility of data contract classes to 'public'
Convert the visibility of data contract converter classes to 'public'
Convert the visibility of request classes to 'public'

Ruhrpottpatriot wrote May 30 at 10:47 AM

Don't do it just yet please. I'm not entirely sure what to do with it and how to handle this.

SkinId is always 0

In the /v2/skins implementation for v1.2.0, the id field is always lost in the conversion from json to objects. As a result, the value of Skin.SkinId is always default(int).

The implementation of /v1/skin_details.json does not have the same problem.

Stop using poor man's dependency injection

[Moved from Codeplex: https://gw2dotnet.codeplex.com/workitem/1346]

While most of the library accepts dependencies as constructor parameters, there are a lot of constructor overloads that create their own objects.

This "technique" is called poor man's dependency injection. Code written like this is only acceptable in the top-level library (GW2NET.csproj).

When you see code like this, refactor it so that there is only 1 constructor that has parameters for every dependency.

class Foo
{
    private readonly IBar bar;

    public Foo()
        : this(new BarImplementation())
    {
    }

    public Foo(IBar bar)
    {
        this.bar = bar;
    }
}

Refactored without poor man's dependency injection:

class Foo
{
    private readonly IBar bar;

    public Foo(IBar bar)
    {
        this.bar = bar;
    }
}

StevenLiekens wrote May 27 at 4:24 PM

Example changeset: https://gw2dotnet.codeplex.com/SourceControl/changeset/40883

StevenLiekens wrote May 28 at 7:52 AM

Recommended reading: https://stackoverflow.com/questions/7099406/what-is-the-real-difference-between-bastard-injection-and-poor-mans-injectio

Replacing *csproj files with *.xproj and project.json files

In the last few days I have learned much about the new ASP.NET project structure. The results were great. The new system offers much more flexibility in coding, while on the same road reducing boilerplate code and useless stuff.

The new ASP.NET 5 project system is built around DI and dynamic references. Publishing to both a stable NuGet and an unstable MyGet feed can be done with one console command.

This will halt some of the work I'm currently doing (mostly refactoring old code into better readable stuff), but it'll be worth it.

Add 'CanConvert'

[Moved from Codeplex: https://gw2dotnet.codeplex.com/workitem/1347]

The IConverter classes currently do not provide fail-safe conversion behavior.

I don't think it is desirable to add fail-safe overloads à la bool TryConvert(..., out result)

Instead, add a method that determines whether the current converter can convert a given value.

interface IConverter<in TInput, out TOutput>
{
    bool CanConvert(TInput value, object state);
    TOutput Convert(Tinput value, object state);
}

With this in place, try-catch blocks are no longer needed.

if (converter.CanConvert(value, null)
{
    var convertedValue = converter.Convert(value, null);
}

Up for grabs

Performance: merge json array with existing list

[Moved from Codeplex: https://gw2dotnet.codeplex.com/workitem/1345]

Inspired by: https://forum-en.guildwars2.com/forum/community/api/Minor-issue-with-X-Result-Total/first#post5103839

The API returns the X-Result-Count of objects with every bulk-expanded response. We should use that value to specify the initial capacity of collection types.

collection.Capacity == X-Result-Count

Unfortunately, Json.NET does not support specifying a capacity for deserialized lists.

Fortunately, Json.NET supports merging json values with an existing object.

TODO (up for grabs)
update json collection converters to first create a new list with a specified capacity, then let Json.NET populate the list with objects

// Pseudo code
var list = new List<TDataContract>(X-Result-Count);
JsonConvert.PopulateObject(list, httpResponseContent);