Map::clipRect populated with weird data

[ldicker83]

Map::clipRect appears to be populated with incorrect or invalid data:

Is this intended? Does this happen with all maps? Should this be zero'd out as it's a field used by the Outpost 2 game but not useful for the end user?

[DanRStevens]

That is peculiar, and looks like either a bug, or bad data that was loaded. Do you have steps to reproduce this?

[Brett208]

Speaking about initializing data structures with default values to ease debugging seems strangely pertinent here.

OP2Utility's map data has only been used for rendering the map, which did not require using the clip rect. I haven't investigated clip rect really at all.

The notes say:

54F814  0x1C    Rect clipRect       [Bound on unit movement]
 54F814  0x1C    int x1 [(lgTileWidth >= 9) ? -1 : 0x20]
 54F818  0x20    int y1 [1]
 54F81C  0x24    int x2 [(lgTileWidth >= 9) ? 2^31-1 : mapTileWidth + 0x1F]
 54F820  0x28    int y2

Our code comment says:

https://github.com/OutpostUniverse/OP2Utility/blob/c23810932cfac1a5926bc4c5b66c5fc8db216461/src/Map/Map.h#L59-L60

This comment is vague and could be improved.

[DanRStevens]

Ahh. Because on Around-The-World maps, the X-coordinate wraps, rather than being clipped. Essentially there is no invalid X-coordinate on those maps.

The value you're seeing is INT_MAX (2**31 - 1, for Outpost2's compile settings).

For non-Around-The-World maps, the contents are shifted right by 32 tiles. I assume to add padding on the left side of the map, to prevent memory access errors if something goes off the side of the map. They also double the memory allocation so you have much more padding on the right side too. It was probably a kludge to work around bounds checking bugs.

---

We need to document that stuff better. We can't exactly change the values, or how it works, so I think we're stuck with documentation.

[ldicker83]

It seemed like it was INT_MAX which is what threw me off as I usually only see that sort of value for uninitialized data in the debugger (and sometimes something like -89193234 or whatever). Anyway, I do remember now that this is expected for around the world maps (I forgot to mention it was one of those).

Would appear this is a non-bug.

[Brett208]

I'd like to consider this reopened this until ClipRect is documented better. Would one of you two be able to push an improved comment on clipRect?

Thanks, Brett

[ldicker83]

I can work on this.

[ldicker83]

Changes committed, please review and close if it looks good.

Note that I'm using doxygen style comments so that we can, in the future, generate offline browse-able documentation.

[Brett208]

@ldicker83, wording looks fine to me. What does the \c mean in the last line of the note? I'm not good with bitwise operations so, glad you are adding.

https://github.com/OutpostUniverse/OP2Utility/blob/1ba5206c8a6b25e0364f7ec09729fae56844fe75/src/Map/Map.h#L59-L66

[ldicker83]

\c is a command used by doxygen to use teletype formatting on the following token. Kind of like the '' in markdown. So when generated the documentation would look like this:

Maps designated 'around the world' allow for continuous scrolling on the X axis and so will populate X1 with -1 and X2 with INT_MAX.

Hope this makes sense.

On that note I would highly highly highly recommend that we use doxygen commenting so that we can generate offline docs: See doxygen usage here: http://www.doxygen.nl/manual/docblocks.html

See http://nas2d.lairworks.com/nas2d_doc/ for an example of a library documented using doxygen.

[Brett208]

I have a very small amount of experience in doxygen. Thanks for the example. I'll open a separate issue to discuss and make sure no problem with moving this way.

-Brett

[DanRStevens]

If only doxygen supported markdown. Oh well.

The new comment looks good. Not much to review. I suppose we can skip the usual branch and pull request for this one.

Small note: Putting "Close #272" or "Closes #272" in the commit message (or opening message of a pull request) should cause GitHub to auto close the issue when the commit is pushed (or merged) to master. Non-keywords will add a reference marker to the commit in the Issue's timeline (such as above).

Thanks for the update Leeor.