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

[Tiled] Work on Object Layers #125

Closed
rafaelalmeidatk opened this issue Feb 11, 2016 · 19 comments
Closed

[Tiled] Work on Object Layers #125

rafaelalmeidatk opened this issue Feb 11, 2016 · 19 comments

Comments

@rafaelalmeidatk
Copy link
Contributor

I need use the object layers but they haven't been implemented yet. How can I work on this? I am trying to understand the TileMapReader.cs but I can't understand the ReadInt32() or other read stuff. Where I begin?

@craftworkgames
Copy link
Collaborator

Okay cool.

So the TiledMapReader and ReadInt32() are part of how custom content importers work in MonoGame / XNA. A while ago I created a tutorial on how to build custom content importers with MonoGame.

Essentially, you'll need to implement the bits for the the Content Pipeline first. For example, the TiledMapReader is paired with the TiledMapWriter in the Content Pipeline project. Every Read operation is paired with a Write operation.

That's how the raw TMX file gets compiled into an XNB file for MonoGame to load when you call Content.Load<TiledMap>("my-map").

Have a go at it and PR changes into the tiled-maps branch please.

@rafaelalmeidatk
Copy link
Contributor Author

Ok, I think I got it. I see that you have done half of the way. How can I debug the TiledMapWriter?

@craftworkgames
Copy link
Collaborator

Yes, debugging these things is rather difficult.

The only way that I've found that works kind of okay is unit tests. I've got some Tiled Map Importer tests already that you should be able to use.

You should be able to put a break point in the test and run it in debug mode to step through the code and see what's going on.

@rafaelalmeidatk
Copy link
Contributor Author

Nice. I have a question. In the TiledMapReaded you iterate through the layers:

for (var i = 0; i < layerCount; i++)
{
    var layer = ReadLayer(reader, tiledMap);
    ReadCustomProperties(reader, layer.Properties);
}

But how do you store them? @edit: wops, forget, already understand it :P

@craftworkgames
Copy link
Collaborator

When you pass a list into a method it gets passed by reference.

    private static void ReadCustomProperties(ContentReader reader, TiledProperties properties)
    {
        var count = reader.ReadInt32();

        for (var i = 0; i < count; i++)
            properties.Add(reader.ReadString(), reader.ReadString());
    }

So in this case, when you call ReadCustomProperties(reader, layer.Properties); it's actually changing the contents of the layer.Properties list. Does that make sense?

In hindsight this is a very strange thing to do in C#. Normally I would frown upon code like this. Maybe it would be better to change it so that ReadCustomProperties returned a list instead. Then the other code would look like this:

layer.Properties = ReadCustomProperties(reader);

Would that make more sense to you?

@craftworkgames
Copy link
Collaborator

I think the reason I did it that way was to maintain a private setter on the Properties properties.

public TiledProperties Properties { get; private set; }

If you did it the other way we'd have to change them to public setters, meaning they could potentially be set to null by accident. Maybe that's not a big deal, but that's the thinking.

@rafaelalmeidatk
Copy link
Contributor Author

The second way is more understandable, but I think the first is acceptable, there's nothing wrong changing directly the list.

@rafaelalmeidatk
Copy link
Contributor Author

I wrote all the stuff to read the objects, but I cant build the .tmx file. Even before I do any change in the code, I receive this error:

An unhandled exception of type 'System.InvalidOperationException' occurred in System.Xml.dll

Additional information: There is an error in XML document (438, 27).

The line 438 of the .tmx is this:

<object id="5" gid="26" x="117.818" y="751.879" width="128" height="128"/>

Context:

<objectgroup name="Objects Layer 1">
 <object id="5" gid="26" x="117.818" y="751.879" width="128" height="128"/>
 <object id="6" gid="1" x="232.97" y="630.667" width="128" height="128"/>
</objectgroup>

EDIT: solved the issue, the x and y are not integers, so I changed the parser to consider as double. EDIT 2: It raised another issue, I will detail on the PR.

@craftworkgames
Copy link
Collaborator

I think the only thing left to do on this is add it to the Tiled Maps demo project.

@rafaelalmeidatk
Copy link
Contributor Author

I think this issue need be fixed:

The values from XML can't contains points.
Example:

<object id="5" gid="26" x="128" y="896" width="297" height="342">

Acceptable.

<object id="5" gid="26" x="88.6061" y="880.848" width="297" height="342">

Unnaceptable, raises an error. I know the error comes from TmxObject.cs, the values aren't integers. If I change from integer to double or float, the values comes wrong. Changing the type of y from int to float:

[XmlAttribute(DataType = "float", AttributeName = "y")]
public float Y { get; set; }

Result:

@craftworkgames
Copy link
Collaborator

Yes, this is very weird. Maybe it's got something to do with how the XML deserializer works? Maybe try doing some googling and see if you can find others with this issue.

@craftworkgames
Copy link
Collaborator

Nevermind. I worked it out.

The problem is actually in TiledMapReader when it's reading the properties from the XNB file. It's expecting them to be int's when they are actually floats.

var x = reader.ReadInt32();
var y = reader.ReadInt32();
var width = reader.ReadInt32();
var height = reader.ReadInt32();

Should be:

var x = reader.ReadSingle();
var y = reader.ReadSingle();
var width = reader.ReadSingle();
var height = reader.ReadSingle();

I'm working on this now. I'll have something checked in shortly.

@craftworkgames
Copy link
Collaborator

I've committed the initial fix but I noticed a number of other properties are not being read and written. In other words, there's still plenty of stuff to do before object layers are fully supported.

I'm going to keep working on this whenever I get spare time.

@craftworkgames
Copy link
Collaborator

Almost everything is done on this now. All of the properties for object layers are being loaded.

The only thing left I'd like to do is add the ability to render the object layers to the screen. Rendering shapes would be an option, but turned off by default.

@HyperionMT
Copy link
Contributor

@craftworkgames, I'm looking to finish up the last bit of work here but wanted to revisit your comment on drawing the objects. I agree we should be rendering any objects created as part of "Insert Tile" on the object layer however it was my interpretation that shapes were not a visual construct and shouldn't be rendered outside of the Tiled editor. I think the fact that there's no way to customize how they appear (color, fill, border thickness) in the Tiled editor supports that. If you still think we should support drawing the shapes I can certainly implement that as well but I wanted to check first.

@craftworkgames
Copy link
Collaborator

If you still think we should support drawing the shapes I can certainly implement that as well but I wanted to check first.

I'd rather not clutter up the renderer with shape rendering code..

However, there is one use case that makes sense to me and that is when you're trying to debug a problem. Now that we have the ability to have multiple renderers I wonder if it might make sense to have a debug renderer separate from the "normal" renderer.

If you're super keep to do it you can, but I'm happy to leave it out for now.

@HyperionMT
Copy link
Contributor

If you're super keep to do it you can, but I'm happy to leave it out for now.

Nope, you pretty much summed up how I feel I just wanted to make sure I did everything required to close this issue. I'll finish up rendering of tiles in object layers and submit a PR.

@HyperionMT
Copy link
Contributor

HyperionMT commented Nov 9, 2016

Ran into an unexpected issue when testing against the Platformer demo. I didn't realize TiledObjectType.Tile objects were being used as spawn points. This seems a little weird to me as you wouldn't want those to display on the map or you'd end up with something like this:
image

Am I just thinking about this wrong? Should I be coming up with a way to deal with this or can we just convert those to shapes?

Edit: When in doubt, make it configurable! So that's what I did, now you can pass in a MapRendererConfig object to turn object layer drawing on/off

@craftworkgames
Copy link
Collaborator

When in doubt, make it configurable! So that's what I did, now you can pass in a MapRendererConfig object to turn object layer drawing on/off

Sorry for the late reply. This is exactly what I was going to say. You beat me to the punch 👍

There's probably going to be lots of creative ways people will want to use the editor. We don't have to support anything and everything but it's nice to make a few things configurable to allow for this.

One the nice things about doing spawn points this way is that you can see in the editor how things are going to look and fit together. It's a nice level editing experience.

There's obviously going to be other reasons people want to place tiles as objects though, so I think you made the right choice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants