patterncsharpMinor
Parsing Valve Map Files (VMF) into a tree like structure
Viewed 0 times
mapintolikevalvefilesparsingstructurevmftree
Problem
I've written a small library of objects for parsing of Valve Map Files (VMF). These files are always generated and exported from either the Hammer World Editor, or a Source engine built in map making program like the PTI in Portal 2. So I can trust them to always look roughly like the following, except of course that these are just bits of what is usually a much more massive file.
in.vmf
I'm looking for feedback on anything relating to structure of my tree, or how I programmed it. If you have any tips, or see some place where I've taken a longer route than necessary, I'd like to know.
I'm not looking for criticism on how my if-else chains look, or how I only include brackets when necessary instead of all the time to maintain consistency. I am more interested in functionality feedback.
One thing I can point out right now, which I believe is inefficient is that when a block is being created, it is looped over just to be collected and passed into anothe
in.vmf
visgroups
{
visgroup
{
"name" "Tree_1"
"visgroupid" "5"
"color" "65 45 0"
}
visgroup
{
"name" "Tree_2"
"visgroupid" "1"
"color" "60 35 0"
visgroup
{
"name" "Branch_1"
"visgroupid" "2"
"color" "0 192 0"
}
visgroup
{
"name" "Branch_2"
"visgroupid" "3"
"color" "0 255 0"
visgroup
{
"name" "Leaf"
"visgroupid" "4"
"color" "255 0 0"
}
}
}
}
viewsettings
{
"bSnapToGrid" "1"
"bShowGrid" "1"
"bShowLogicalGrid" "0"
"nGridSpacing" "64"
"bShow3DGrid" "0"
}
side
{
"id" "6"
"plane" "(512 -512 -512) (-512 -512 -512) (-512 -512 512)"
"material" "BRICK/BRICKFLOOR001A"
"uaxis" "[1 0 0 0] 0.25"
"vaxis" "[0 0 -1 0] 0.25"
"rotation" "0"
"lightmapscale" "16"
"smoothing_groups" "0"
dispinfo
{
}
}I'm looking for feedback on anything relating to structure of my tree, or how I programmed it. If you have any tips, or see some place where I've taken a longer route than necessary, I'd like to know.
I'm not looking for criticism on how my if-else chains look, or how I only include brackets when necessary instead of all the time to maintain consistency. I am more interested in functionality feedback.
One thing I can point out right now, which I believe is inefficient is that when a block is being created, it is looped over just to be collected and passed into anothe
Solution
public VProperty(string name, string value = "")
public VProperty(string text)This means that the default value will never be used. If you write
new VProperty("propertyName"), it will call the text overload.To avoid the confusion, consider using some way of differentiate parsing from normal constructor, like using a static factory method (e.g.
public static VProperty Parse(string text)), or even have the parsing and possibly also ToVMFString() in a separate class.'\"'You don't need the backslash there,
'"' works fine.public VBlock(string name, IList body = null)
{
Name = name;
if (body == null)
body = new List();
}This constructor won't work, you never actually assign to
Body.You can make members of internal classes public, instead of internal. It will work exactly the same, but it will make it much simpler to make the class public, if you ever decide to do that.
IList nBody = new List();
int depth = 0;
var wasDeep = false;The name
nBody is not very good. What does the n mean? You shouldn't use abbreviations like that to make your code clearer.Also, there is no need to specify the type for
nBody, you could use var here. On the other hand, I would be explicit with wasDeep, I think saving that one character is not worth it.Code Snippets
public VProperty(string name, string value = "")
public VProperty(string text)public VBlock(string name, IList<IVNode> body = null)
{
Name = name;
if (body == null)
body = new List<IVNode>();
}IList<IVNode> nBody = new List<IVNode>();
int depth = 0;
var wasDeep = false;Context
StackExchange Code Review Q#44895, answer score: 8
Revisions (0)
No revisions yet.