marshallward / TiledSharp

C# library for parsing and importing TMX and TSX files generated by Tiled, a tile map generation tool
http://marshallward.github.io/TiledSharp
Apache License 2.0
329 stars 87 forks source link

Typo in TmxObject ctor: TmxLayerTile ctor gets passed X position twice #61

Closed hu9o closed 5 years ago

hu9o commented 5 years ago

While browsing the code, I came across what seems to be a typo:

https://github.com/marshallward/TiledSharp/blob/f29fb71591200093fa159f53094b8b8d7fab1d17/TiledSharp/src/ObjectGroup.cs#L99-L101

X (rounded) is passed as argument to both the x and y params.

I just started using Tiled and TiledSharp, so I'm not sure what I'm looking at and I haven't encountered any bug caused by that -- it just looks very suspect.

marshallward commented 5 years ago

I think you're right, it does look like a bug. Would you like to send a PR? Otherwise I can make the change.

On Fri, Aug 23, 2019 at 8:40 PM Hugo Ratiney notifications@github.com wrote:

While browsing the code, I came across what seems to be a typo:

https://github.com/marshallward/TiledSharp/blob/f29fb71591200093fa159f53094b8b8d7fab1d17/TiledSharp/src/ObjectGroup.cs#L99-L101

X (rounded) is passed as argument to both the x and y params.

I just started using Tiled and TiledSharp, so I'm not sure what I'm looking at and I haven't encountered any bug caused by that -- it just looks very suspect.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/marshallward/TiledSharp/issues/61?email_source=notifications&email_token=AADQ326QA5U3IP2S7RJJTITQGB7PRA5CNFSM4IPEUZAKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HHE2PLA, or mute the thread https://github.com/notifications/unsubscribe-auth/AADQ327XPVDMPYW4GRFTBHDQGB7PRANCNFSM4IPEUZAA .

marshallward commented 5 years ago

My guess is that no one had noticed this because that tiles as objects is relatively obscure and is probably not being used by anyone. (That and I don't really invest much time into this project, particularly testing!)

hu9o commented 5 years ago

I hadn't PRed because I assumed it would require unit tests that I don't really feel confident writing, since I don't really know Tiled.

But after checking out the test project, I realize there's much work to be done there... so here's a quick PR in the meantime.

marshallward commented 5 years ago

Yeah, this project is mostly on low-level maintenance mode... I haven't touched Tiled in years but I try to take feedback from people to keep it running.

Thanks very much for the PR, I'll move it in now.