0

Sorry for the messy title, I'm not entirely sure how to articulate it.

I'm trying to use a foreach loop for a script in my unity project for a sort of line-generator on a grid. Essentially, I have an origin point on the grid, and I want all of the points that have either the same X-axis or the same Y-axis, so I end up with a cross shape. Here's a rough draft of what I've come up with:

    public void MarkStraightTiles(Cell cell)
    {
        neighbourCells = Origin.GetNeighbours(allCells);
        foreach(var neighbour in neighbourCells)
        {
            if(neighbour.OffsetCoord.x == Origin.OffsetCoord.x || neighbour.OffsetCoord.y == Origin.OffsetCoord.y)
            {
                neighbourCells.AddRange(neighbour.GetNeighbours(allCells));
            }
        }
    }

For reference, GetNeighbours just returns a list of the four points around Origin from allCells. OffsetCoord is just the position of the point. In theory, I want it so that for every Origin neighbour, I get the neighbour of the neighbour, and repeat until I have every point with either the same X axis or Y axis. Obviously the code above doesn't work, and I don't know how to repeat the foreach loop with every new neighbour I get. I can't modify the foreach loop while it's running, it throw up an error. Any ideas on this?

derHugo
  • 83,094
  • 9
  • 75
  • 115
  • 1
    `cell` isn't used and `allCells` isn't defined. Did you post incorrect code? – Enigmativity Mar 16 '23 at 02:19
  • According to https://stackoverflow.com/a/759985/2051953 you cannot modify the collection used in a foreach; You need a for-loop – agbinfo Mar 16 '23 at 02:27
  • Sorry, 'cell' is a class from a framework, and 'allCells' is just a list of all current cells on the graph – Wigglepuff Mar 16 '23 at 02:38
  • How are `cell` and `Origin` related and how exactly does `GetNeighbours(allCells)` work? Can't you just take an entire line and simply return all cells that match the same X/Y coordinate without any recursion through neighbors? Simply e.g. `return allCells.Where(c => c.OffsetCoord.x == cell.OffsetCoord.x || c.OffsetCoord.y == cell.OffsetCoord.y).ToArray();` – derHugo Mar 16 '23 at 08:31
  • Removing the `unity3d` tag as this is not bound to any Unity specific API but rather a a general c# question – derHugo Mar 16 '23 at 08:45

4 Answers4

0

This doesn't exactly answer your question.. but if understand correctly what you're asking.. you'd want something like:

allCells.Where(p=>p.OffsetCoord.x == cell.OffsetCoord.x);

i don't actually know what objects your working with so may or not be anything like that.. but if you want to do what you said, create a new array or list and add it directly to the new list instead of the one you're iterating through..

    public void MarkStraightTiles(Cell cell)
    {
        neighbourCells = Origin.GetNeighbours(allCells);
        var newlist = neighbourCells.ToList();
        foreach(var neighbour in neighbourCells)
        {
            if(neighbour.OffsetCoord.x == Origin.OffsetCoord.x || neighbour.OffsetCoord.y == Origin.OffsetCoord.y)
            {
                newlist.AddRange(neighbour.GetNeighbours(allCells));
            }
        }
    }
Mewers
  • 1
  • 1
  • I also think OP actually simply wants the `allCells.Where(p=>p.OffsetCoord.x == cell.OffsetCoord.x || p.OffsetCoord.y == cell.OffsetCoord.y);` ... your second snippet isn't recursive and just yields the second level neighbors .. including duplicates since the `neighbour.GetNeighbours` probably again contain the original cell as well ;) – derHugo Mar 16 '23 at 08:37
0

You probably need something recursive like this:

public IEnumerable<Cell> RecurseCells(IEnumerable<Cell> cells)
{
    foreach (var neighbour in cells)
    {
        yield return neighbour;
        if (neighbour.OffsetCoord.x == Origin.OffsetCoord.x || neighbour.OffsetCoord.y == Origin.OffsetCoord.y)
        {
            foreach (var remainder in RecurseCells(Origin.GetNeighbours(neighbour)))
            {
                yield return remainder;
            }
        }
    }
}

public void MarkStraightTiles(Cell cell)
{
    foreach (var neighbour in RecurseCells(Origin.GetNeighbours(cell)))
    {
        /* Do whatever with neighbours */
    }
}
Enigmativity
  • 113,464
  • 11
  • 89
  • 172
  • In the inner `foreach` did you rather mean to use `GetCells`? Otherwise this wouldn't be recursive but only yield second level neighbors or do I get this wrong? You would probably also need to exclude neighbors you already yielded before .. otherwise it would loop forth and back between the neighbors. Tbh I also don't see the relationship between the passed `cell`, `Origin` and `allCells` in OP's code ... – derHugo Mar 16 '23 at 08:28
  • @derHugo - Yes, indeed - `GetCells(neighbour)` in fact. THat's why I like real code that I can test. The OP posted incorrect code so that's the quality my answer rose to. – Enigmativity Mar 16 '23 at 08:50
  • @derHugo - No, it's still wrong. – Enigmativity Mar 16 '23 at 08:51
  • Yeah we still lack more information about what `cell` and `Origin` etc does exactly ... I kinda guess actually `cell == Origin` and OP has some weird `GetNeighbours` method iterating the entire collection of `allCells` in order to find the neighbors .. and ofcourse never caches them :'D – derHugo Mar 16 '23 at 08:52
  • The issue is if I understood correctly OP only wants to go in straight lines from the original cell .. so we don't want to return **all** neighbors of the recursive neighbors but only those being on the same "path" in either +/- X/Y directions. You would probably just need more parameters to handle this ... Tbh I still tend to believe though that what OP **actually** wants is either simply iterate the "grid" as [here](https://stackoverflow.com/a/75751707/7111561) or simply use Linq like [here](https://stackoverflow.com/a/75751604/7111561) – derHugo Mar 16 '23 at 08:56
0

I wouldn't use GetNeighbors() at all. We have knowledge of vertical and horizontal lines on a Cartesian plane. Given a 2D grid and a point on it to get a cross, the code would be (setting aside where things like minX come from)

// small performance boost: pre-allocate the list so it won't resize
var cross = new List<Cell>(maxX + maxY);
for (int x=minX; x<=maxX; x++)
{
    // add all cells along the X axis at given Y coordinate    
    cross.Add(x, origin.y);
}
for (int y=minY; y<=maxY; y++)
{
    // add all cells along the Y axis at given X coordinate    
    cross.Add(origin.x, y);
}

This avoids recursion, LINQ, and scanning the entire grid O(maxX * maxY) vs OMaxX + maxY).

If you need the cross without the origin, you can change the loops to

for (int x=minX; x<=maxX; x++)
{
    // add all cells along the X axis at given Y coordinate except self
    if (x == origin.x) continue;
    cross.Add(x, origin.y);
}
for (int y=minY; y<=maxY; y++)
{
    // add all cells along the Y axis at given X coordinate except self   
    if (y == origin.y) continue;
    cross.Add(origin.x, y);
}
Kit
  • 20,354
  • 4
  • 60
  • 103
  • Fixed a typo and then thought why not directly use according variable name ;) Then I thought: Why not simply have a nested loop, and do both iterations in a go? `var neighbors = new List((maxX-minX) + (maxY-minY)); for(var x = minX; x <= maxX; x++){ for(var y = minY; y <= maxY; y++) { if(x == origin.x && y == origin.y) { continue; } if(x == origin.x || y == origin.y) { neighbors.Add(x, y); } } }` (actually OP's code doesn't use any Unity specific API ;) ) – derHugo Mar 16 '23 at 08:44
0

Essentially, I have an origin point on the grid, and I want all of the points that have either the same X-axis or the same Y-axis, so I end up with a cross shape. Here's a rough draft of what I've come up with

So you have a starting point and want all points with either same x or same y coord? In that case shouldn't this be sufficient: var neighbourCells = allCells.Where(cell => cell.OffsetCoord.x == Origin.OffsetCoord.x || cell.OffsetCoord.y == Origin.OffsetCoord.y).ToList();

Or are you specifically asking for a foreach/for implementation?

Drewfyre
  • 1
  • 1
  • That would work, but your algorithm requires every cell to be visited, even though we know we don't have to. – Kit Mar 16 '23 at 13:28
  • That is true. The viability of this approach depends on how big this grid is or how often you would execute it. The bigger the grid the slower this approach. – Drewfyre Mar 16 '23 at 21:47