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

WangTiles now supports infinite maps #1688

Merged
merged 2 commits into from
Aug 16, 2017
Merged

Conversation

Bdtrotte
Copy link
Contributor

Fixed tests as well

Copy link
Member

@bjorn bjorn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can simplify this a lot. See comments. :-)

@@ -544,7 +544,7 @@ void WangBrush::updateBrush()
continue;

QPoint p = adjacentPositions[i];
if (!currentLayer->contains(p) || currentLayer->cellAt(p).isEmpty())
if ((!currentLayer->contains(p) && !mapDocument()->map()->infinite()) || currentLayer->cellAt(p).isEmpty())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can drop the (!currentLayer->contains(p) && !mapDocument()->map()->infinite()) || part now, since just currentLayer->cellAt(p).isEmpty() will work in both cases.

@@ -622,7 +622,7 @@ void WangBrush::updateBrush()
for (int i = 0; i < 4; ++i) {
QPoint p = adjacentPoints[i];

if (!currentLayer->contains(p) || currentLayer->cellAt(p).isEmpty())
if ((!currentLayer->contains(p) && !mapDocument()->map()->infinite()) || currentLayer->cellAt(p).isEmpty())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can also be just if (currentLayer->cellAt(p).isEmpty()).

@@ -681,7 +681,7 @@ void WangBrush::updateBrush()
dirPoint = mPaintPoint + aroundTilePoints[mEdgeDir*2];
}

if (currentLayer->contains(mPaintPoint)) {
if (mapDocument()->map()->infinite() || currentLayer->contains(mPaintPoint)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the code nested in this condition actually only works on non-empty cells, the above condition can be dropped entirely. Actually, you can also drop the !currentLayer->cellAt(mPaintPoint).isEmpty() check, since in that case the wangId is always 0 anyway.

Same for the below code dealing with dirPoint.


QPointF point = renderer.screenToTileCoords(screenCoords);

QCOMPARE(QPoint(qFloor(point.x()), qFloor(point.y())), tileCoords.toPoint());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, should have noticed that. Thanks for fixing!

@@ -285,7 +287,7 @@ WangId WangFiller::wangIdFromSurroundings(const TileLayer &back,
getSurroundingPoints(point, mStaggeredRenderer, mStaggerAxis, adjacentPoints);

for (int i = 0; i < 8; ++i) {
if (!fillRegion.contains(adjacentPoints[i]) && back.contains(adjacentPoints[i]))
if (!fillRegion.contains(adjacentPoints[i]) && (mIsInfinite || back.contains(adjacentPoints[i])))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since cellAt will just return an empty cell and no longer asserts when the position is outside of the layer, the && (mIsInfinite || back.contains(adjacentPoints[i])) part can be left out entirely.

return back.cellAt(point);
} else if (front.contains(point.x() - front.x(), point.y() - front.y())) {
} else if ((mIsInfinite || front.contains(point.x() - front.x(), point.y() - front.y()))) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check can actually be dropped entirely, so we can change this to else and drop the current else part.

@@ -248,9 +250,9 @@ const Cell &WangFiller::getCell(const TileLayer &back,
const QRegion &fillRegion,
QPoint point) const
{
if (!fillRegion.contains(point) && back.contains(point)) {
if (!fillRegion.contains(point) && (mIsInfinite || back.contains(point))) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Outside the fill region I guess we should always take the tile from back? Cause in that case we can drop the rest of the condition.

@bjorn bjorn merged commit 2384dce into mapeditor:master Aug 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants