Feature Requests and Bug Reports

Developer
Sep 12, 2014 at 1:13 PM
Please use this thread to submit feature requests. Found a bug? Create a new work item instead.
Coordinator
Sep 22, 2014 at 6:33 PM
This is a request made by me and will be done by. Just putting it here so I don't forget it.

The xml documentation is in dire need of updates. I just built website with sandcastle and most of the comments are pretty useless. I'll see what I can do in the next days and weeks.
Developer
Oct 18, 2014 at 12:08 PM
Edited Oct 18, 2014 at 12:10 PM
Related: should we always write documentation for internal types? Think stuff like DTO classes that are only used internally. Code like that should not require an explanation for why it exists or even what it is used for. I'm not saying that internal code never requires documentation, but we do have a lot of internal files where xml doc comments do nothing except get in the way of the programmer who has to maintain the actual code.
Coordinator
Oct 18, 2014 at 2:01 PM
I'm a bit conflicted about this topic. On one hand you are right, that comments could get in the way. However there is also the possibility that a programmer does not know what the method/class actually does (yes,yes, I know... code should be self explanatory. But for what do we have intellisense then?)

I'll look into it, but currently I lean towards more comments.
Developer
Oct 18, 2014 at 2:30 PM
Well, it doesn't have to be all or nothing. Here's an example of a class where I added some helpful reminders, but without going overboard with the useless comments.

https://gw2dotnet.codeplex.com/SourceControl/latest#GW2.NET/Code/GW2NET.Core/V2/Items.Json/DetailsDataContract.cs
Developer
Nov 9, 2014 at 7:49 AM
Feature request: color converters for <c> tags in item descriptions

Example: Chaos of Lyssa
<c=@flavor>"Many have eyes, but few have seen. Of all here, you saw the beauty behind the illusion. And you alone shall be blessed with My gifts." - Scriptures of Lyssa, 45 BE<c>
You could write your own rendering engine that understands <c> tags, but typically you would convert them to whatever markup language you were already using to display content. Either way, you first you have to separate the color information from the content. I think that the library should be able to do that for you.

Does C# have any sort of RichTextString class that we could use for this? Something that isn't specific to winforms, obviously.
Coordinator
Nov 11, 2014 at 5:31 PM
Does C# have any sort of RichTextString class that we could use for this? Something that isn't specific to winforms, obviously.
Not that I know of. The only RichText I'm aware of is the RichTextBox in WPF. However personally I never had to use RichText. usually I worked with plain strings. This is my take on C# too. In WinForms/WPF you'd rarely need RichText, except you are writing a RichText editor. For web you'd use simple strings and format/style them with CSS.
As for the converter: Since this seems like xml to me, we could write a POCO like this:
    CTag
    {
        public string[] Tags {get; set;}
        
        public string Content {get; set;}
    }
The we'd convert the tag into said POCO and store it however we please.
Developer
Nov 12, 2014 at 7:22 AM
It's not xml, because then it would have looked like this: <tag c="@flavor">. I do wonder if its current form (<c=@flavor>) is an existing markup language, or a language that they invented for the game. I've never seen it before, so I'm guessing it's custom code.
Coordinator
Nov 12, 2014 at 2:44 PM
So I just wrote a quick parser. It's pretty basic but I think with some optimization we could use it.
namespace GuildEmblemParser
{
    using System;
    using System.Text.RegularExpressions;

    public class StringParser
    {
        public StringParser(string stringToParse)
        {
            this.StringToParse = stringToParse;
        }

        public string StringToParse { get; private set; }

        public virtual FlavorText ParseText()
        {
            Regex flavouRegex = new Regex("<c=@flavor>.*<c>", RegexOptions.Singleline);

            string match = flavouRegex.Match(this.StringToParse).Captures[0].Value;
            
            string firstTag = match.Substring(0, match.IndexOf(">", StringComparison.Ordinal) + 1);

            int lastIndexOfLt = match.LastIndexOf("<", StringComparison.Ordinal);

            string lastTag = match.Substring(lastIndexOfLt, 3);

            string[] tagsArray = 
            {
                firstTag, 
                lastTag
            };

            int textLength = lastIndexOfLt - match.IndexOf(">", StringComparison.Ordinal) - 1;

            string flavourText = match.Substring(match.IndexOf(">", StringComparison.Ordinal) + 1, textLength);

            return new FlavorText 
            { 
                Tags = tagsArray,
                Text = flavourText 
            };
        }
    }

    public class FlavorText
    {
        /// <summary>
        /// Backing field for the tags property.
        /// </summary>
        private string[] tags;

        /// <summary>
        /// Gets or sets the tags of the flavor text, first item is always the start tag, second the end tag.
        /// </summary>
        public string[] Tags
        {
            get
            {
                return this.tags;
            }
            set
            {
                if (value.Length > 2)
                {
                    throw new ArgumentException("Length of the array cannot be greater than two.");
                }

                this.tags = value;
            }
        }

        /// <summary>
        /// Gets or sets the flavor text
        /// </summary>
        public string Text { get; set; }
    }
}
Developer
Nov 12, 2014 at 4:32 PM
Edited Nov 12, 2014 at 4:33 PM
I just remembered that I did something similar for the demo site 6 months ago. I should probably update/delete those pages. The demo doesn't represent the library in its current state at all.

https://github.com/StevenLiekens/GW2.NET-Demo/blob/master/GuildWars/Views/Items/_ItemRow.cshtml#L49
(gah... write-only code)
Coordinator
Nov 12, 2014 at 7:58 PM
Yeah it is something similar, but it only gets the description not the tag type. I'll refactor my code tomorrow so taht we could use it for your usage scenarios too.
Developer
Nov 13, 2014 at 7:49 AM
Edited Nov 13, 2014 at 8:28 AM
Chaos of Lyssa was probably not the best example. Its entire description is the same color. We have to support combinations of plain text and colored text. Possibly even multiple color tags in a single item description.
 
This combination of plain text and <c=@flavor>colored text<c> is valid. <c=@warning>Multiple tags are also valid.<c>
 
I'm thinking we could associate class FlavorText with a StartIndex and Length to mark a substring.
class FlavorText
{
    int[] RGB { get; set; };
    
    int StartIndex { get; set; };
    
    int Length { get; set; };
}
Then, add our own RichTextString class that wraps the text along with a collection of FlavorText objects that represent the markup.
class RichTextString
{
    string Text { get; set; };
    
    FlavorText[] Markup { get; set; };
}
Example: Chaos of Lyssa
Text = "Many have eyes, but few have seen. Of all here, you saw the beauty behind the illusion. And you alone shall be blessed with My gifts." - Scriptures of Lyssa, 45 BE
Markup[0] = { StartIndex = 0, Length = 164, RGB = #FFADD8E6 }
This is definitely the most flexible solution. It even allows for nested tags. It's also the most difficult to implement. Regular expressions are too limited for determining index and length. We'll need a custom lexer and parser for this.
Coordinator
Nov 13, 2014 at 7:02 PM
Why are regular expressions not flexible enough? Each tag starts with <c=@some text and ends with <c>. Regex can definitely parse it, as the following code sample shows:
using System;
    using System.Collections.Generic;
    using System.Text.RegularExpressions;

    public class StringParser
    {
        /// <summary>
        /// Gets the tagged substrings fro a specified string.
        /// </summary>
        /// <returns>
        /// An <see cref="IEnumerable{T}"/> of all subsstrings with their type.
        /// </returns>
        public virtual IEnumerable<FlavorText> GetSubstrings(string stringToParse)
        {
            Regex flavouRegex = new Regex("<c=@.+?>.+?<c>", RegexOptions.Singleline);

            MatchCollection matches = flavouRegex.Matches(stringToParse);

            foreach (Match regexMatch in matches)
            {
                string match = regexMatch.Value;

                int firstLtIdx = match.IndexOf(">", StringComparison.Ordinal);

                int atSignIdx = match.IndexOf("@", StringComparison.Ordinal);

                string flavorType = match.Substring(atSignIdx + 1, (firstLtIdx - atSignIdx) - 1);
                
                int lastGtIdx = match.LastIndexOf("<", StringComparison.Ordinal);

                string flavorText = match.Substring(firstLtIdx + 1, (lastGtIdx - firstLtIdx) - 1);

                yield return new FlavorText { TagType = flavorType, Text = flavorText };
            }
        }
    }

    public class FlavorText
    {
        /// <summary>
        /// The type of the text.
        /// </summary>
        public string TagType { get; set; }

        /// <summary>
        /// Gets or sets the flavor text
        /// </summary>
        public string Text { get; set; }
    }
The test I have written also runs correctly for this text: "This combination of plain text and <c=@flavor>colored text<c> is valid. <c=@warning>Multiple tags are also valid.<c>"
It spits out two Flavor Text object with
  1. flavor as TagType and colored text as Text
  2. warning as TagType and Multiple tags are also valid. as Text
I could enhance the parser further so we can get the original text without tags and flavouring, should be easy enough. Anything else I should add?
Developer
Nov 13, 2014 at 7:10 PM
Edited Nov 13, 2014 at 7:14 PM
Ideally I'd like to keep the entire item description in a single string, then provide the markup information separately. That's why I came up with the StartIndex and Length properties for each tag. Regex can't give you those numbers.
Text      = "This combination of plain text and colored text is valid. Multiple tags are also valid."
Markup[0] = { StartIndex = 35, Length = 12, TagType = "@flavor" }
Markup[1] = { StartIndex = 58, Length = 29, TagType = "@warning" }
Coordinator
Nov 13, 2014 at 7:14 PM
Edited Nov 13, 2014 at 7:15 PM
Well that is possible too (not with Regex maybe), but I'll think of something later today.
Developer
Nov 13, 2014 at 7:22 PM
Yeah we need something that reads the description one character at a time, updating the current position after each character. When an opening tag is reached, the current position is the StartIndex for that tag. When a closing tag is reached, calculate the difference between the current position and the StartIndex to get the Length value.
Coordinator
Nov 14, 2014 at 1:15 PM
Edited Nov 14, 2014 at 1:16 PM
Regex in C# gives us the information we need. If you use Matches instead of IsMatch you get an Index and a Length property. I fiddled around with it a bit and came up with a better solution. Ironically the idea came from a ConsoleApplication. My code is a slightly more complicated version than our idea, but I think it will grant us additional freedom on the later run. This is mainly the reason why I didn't store the RGB Values inside the FlavorText object, since maybe the end users wants his own colouring. That said, it is totally possible to implement your idea with regex.

Here is what I did:
  1. Take the string to parse and store it inside RichTextString object, alongside with the number of overall matches in the text.
  2. Iterate over the MatchesCollection and replace every match with an placeholder that has the following format {index}, whereas the index the index of the current match is.
  3. Convert the match to the FlavorText object and store it at the same index inside the FlavorText[] in the RichTextString object.
  4. Return the RichTextObject
For our example String we get the following output:
Text = This combination of plain text and {0} is valid. {1}
Markup:
  • { Type = "flavor", Value = "colored text" }
  • { Type = "warning", Value = "Multiple tags are also valid." }
Anyway here is the code I wrote:
 using System.Text.RegularExpressions;

    /// <summary> Contains methods to parse a arbitrary string to a <see cref="RichTextString"/>. </summary>
    public class StringParser
    {
        /// <summary> Parses a string to a <see cref="RichTextString"/>. </summary>
        /// <param name="textToParse">The text to parse.</param>
        /// <returns>The <see cref="RichTextString"/>.</returns>
        public RichTextString Parse(string textToParse)
        {
            MatchCollection matches = Regex.Matches(textToParse, @"<c=@(.+?)>(.+?)<c>");

            RichTextString richTextString = new RichTextString(matches.Count, textToParse);

            int index = 0;

            foreach (Match match in matches)
            {
                richTextString.Text = richTextString.Text.Replace(match.Value, "{" + index + "}");

                richTextString.Markup[index] = new FlavorString
                {
                    Type = match.Groups[1].Value,
                    Value = match.Groups[2].Value
                };

                index++;
            }

            return richTextString;
        }
    }

    /// <summary>Represents a rich text string.</summary>
    /// <remarks>This object represents a text string, that has some parts adorned with additional style information.
    /// These information have been cut out from the string and been stored inside the <see cref="Markup"/> property.
    /// The index of each information is equal to the index in the Markup property array.</remarks>
    public class RichTextString
    {
        /// <summary>Initializes a new instance of the <see cref="RichTextString"/> class.</summary>
        /// <param name="numberOfFlavorTexts">The total number of flavor text objects inside the string.</param>
        /// <param name="text">The complete text.</param>
        public RichTextString(int numberOfFlavorTexts, string text)
        {
            this.Markup = new FlavorString[numberOfFlavorTexts];
            this.Text = text;
        }

        /// <summary>Gets or sets the rich text.</summary>
        public string Text { get; set; }

        /// <summary>Gets or sets the array containing markup information.</summary>
        public FlavorString[] Markup { get; set; }
    }

    /// <summary>Represents a flavored string.</summary>
    public class FlavorString
    {
        /// <summary>Gets or sets the type of the flavor text.</summary>
        public string Type { get; set; }

        /// <summary>Gets or sets the value of the flavor text.</summary>
        public string Value { get; set; }
    }
}
Coordinator
Nov 14, 2014 at 2:08 PM
I might add, that this Parser still needs some work. E.g. a ToString() that returns the whole text, without placeholders and such.