Danny May
Danny May
PostsComments
CC#
Created by Mąż Zuzanny Harmider Szczęście on 1/28/2025 in #help
✅ World generation optimization
No clue off the top of my head, but again would be a case of adding logs and timestamps to other areas of your application to see if theres anything else which is getting called during that period. Pretty sure that Loaded is one of the last events to get called, so the framework will have been doing a lot in the meantime. Also seeing as this is called MainWindow, im guessing this is the window that shows up as the first thing after you started the application? Probably not surprising that it takes a second or two to warm up
104 replies
CC#
Created by Mąż Zuzanny Harmider Szczęście on 1/28/2025 in #help
✅ World generation optimization
Looks like theres a ~1s delay between the component being constructed and the Loaded event being triggered. Im not sure what would be causing that, but it might just be how long it takes for the framework to initialize
104 replies
CC#
Created by Mąż Zuzanny Harmider Szczęście on 1/28/2025 in #help
✅ World generation optimization
Might be just how long it takes to load the images
104 replies
CC#
Created by Mąż Zuzanny Harmider Szczęście on 1/28/2025 in #help
✅ World generation optimization
Put a checkpoint before and after the TileImages= line
104 replies
CC#
Created by Mąż Zuzanny Harmider Szczęście on 1/28/2025 in #help
✅ World generation optimization
best way to solve performance issues is to measure, measure, measure
104 replies
CC#
Created by Mąż Zuzanny Harmider Szczęście on 1/28/2025 in #help
✅ World generation optimization
Like I said, loop over the tiles directly rather than their positions 
var availableTiles = Display.Children.OfType<Tile>().OrderBy(x => random.Next()).ToList();
foreach (var existingTile in availableTiles)
{
  ...
}
var availableTiles = Display.Children.OfType<Tile>().OrderBy(x => random.Next()).ToList();
foreach (var existingTile in availableTiles)
{
  ...
}
But you need to use the stopwatch to work out what is taking the longest, I dont see anything off the top of my head that jumps out as an issue
104 replies
CC#
Created by Mąż Zuzanny Harmider Szczęście on 1/28/2025 in #help
✅ World generation optimization
Wont know without benchmarking.
104 replies
CC#
Created by Mąż Zuzanny Harmider Szczęście on 1/28/2025 in #help
✅ World generation optimization
1kms to generate 2k tiles feels quite high to me. If youre ok with that though then is there still an issue you need help with?
104 replies
CC#
Created by Mąż Zuzanny Harmider Szczęście on 1/28/2025 in #help
✅ World generation optimization
but again, at 2k elements total the issue is likely somewhere else
104 replies
CC#
Created by Mąż Zuzanny Harmider Szczęście on 1/28/2025 in #help
✅ World generation optimization
The .Any will still loop over the children until it matches, so you have a loop within a loop of the same collection making it O(n^2) rather than O(n)
104 replies
CC#
Created by Mąż Zuzanny Harmider Szczęście on 1/28/2025 in #help
✅ World generation optimization
lines 139, 140 & 147 in the scriptbin you sent
104 replies
CC#
Created by Mąż Zuzanny Harmider Szczęście on 1/28/2025 in #help
✅ World generation optimization
Youll have to change all your Display.Children.Add to call AddTilesToUIThread though
104 replies
CC#
Created by Mąż Zuzanny Harmider Szczęście on 1/28/2025 in #help
✅ World generation optimization
Not what I expected but sure. Try the Task.Run thing anyway, the window should pop up almost immediately while GenerateWorld runs in the background
104 replies
CC#
Created by Mąż Zuzanny Harmider Szczęście on 1/28/2025 in #help
✅ World generation optimization
Looking over the code I cant see anywhere that jumps out as a performance bottleneck, not at 2k items in the collection. Worst case this line will have to loop through all 2k tiles for every tile you attempt to place a mushroom or tree at 
var existingTile = Display.Children.OfType<Tile>().FirstOrDefault(t => t.X == col * Tile.Size && t.Y == row * Tile.Size);
var existingTile = Display.Children.OfType<Tile>().FirstOrDefault(t => t.X == col * Tile.Size && t.Y == row * Tile.Size);
If you instead looped over a shuffled list of tiles instead of their positions, you wouldnt have to do a lookup at all 
var availableTiles = Display.Children.OfType<Tile>().OrderBy(x => random.Next()).ToList();
foreach (var existingTile in availableTiles)
{
  ...
}
var availableTiles = Display.Children.OfType<Tile>().OrderBy(x => random.Next()).ToList();
foreach (var existingTile in availableTiles)
{
  ...
}
104 replies
CC#
Created by Mąż Zuzanny Harmider Szczęście on 1/28/2025 in #help
✅ World generation optimization
Id recommend you split GenerateWorld up into multiple methods too, theres a lot going on in there.
104 replies
CC#
Created by Mąż Zuzanny Harmider Szczęście on 1/28/2025 in #help
✅ World generation optimization
Considering there are only 2160 tiles (I think) 3s is a lot of time for generating the world. I think the change to the time images will help, but probably not solve the speed issue. AddTilesToUIThread wont really help much I dont think because GenerateWorld is already running on the UI thread, so they will be dispatched instantly. A quick and dirty way to get GenerateWorld to run in the background would be to change 
Loaded +=(s,e)=>GenerateWorld();
Loaded +=(s,e)=>GenerateWorld();
to 
Loaded +=(s,e)=> Task.Run(GenerateWorld);
Loaded +=(s,e)=> Task.Run(GenerateWorld);
104 replies
CC#
Created by Mąż Zuzanny Harmider Szczęście on 1/28/2025 in #help
✅ World generation optimization
I dont think the answer would be to make it async here, atleast not at first. I think theres some issues with the loops in the first place, depending on the size of WorldHeight and WorldWidth. In the first loop of GenerateWorld you loop over every tile (WorldHeight * WorldWidth iterations) and create a new tile. Inside the Tile constructor, you then loop over the Display.Children twice to find a tile with the same type and null source. It would be much faster to initialize your image sources first, and then look them up in a dictionary. 
class MainWindow
{
    public Dictionary<TileType, BitmapImage> TileImages { get; private set; }

    public MainWindow()
    {
        InitializeComponent();
        TileImages = Enum.GetValues<TileType>()
            .ToDictionary(type => type, type => new BitmapImage(new Uri(GetImagePath(type), UriKind.RelativeOrAbsolute)));
        ...
    }
}

class Tile 
{
    public Tile(MainWindow window, double x, double y, TileType type) : this(window)
    {
        X = x;
        Y = y;
        Type = type;
        Source = window.TileImages[type];
        MouseLeftButtonDown += TileClicked;
    }
}
class MainWindow
{
    public Dictionary<TileType, BitmapImage> TileImages { get; private set; }

    public MainWindow()
    {
        InitializeComponent();
        TileImages = Enum.GetValues<TileType>()
            .ToDictionary(type => type, type => new BitmapImage(new Uri(GetImagePath(type), UriKind.RelativeOrAbsolute)));
        ...
    }
}

class Tile 
{
    public Tile(MainWindow window, double x, double y, TileType type) : this(window)
    {
        X = x;
        Y = y;
        Type = type;
        Source = window.TileImages[type];
        MouseLeftButtonDown += TileClicked;
    }
}
Then in the MaxTrees and MaxMushrooms loops of GenerateWorld you iterate and over all the tiles on every iteration, but never change any property on the tile in either of the loops. Pull these lines out of the for loops: 
Tile[] tiles = Display.Children.OfType<Tile>().Where(r => r.Type == TileType.GrassA).ToArray();
for(int i = 0; i < MaxTrees; i++)
{
   ...
}
tiles = tiles.Where(tile =>!tiles.Any(otherTile => tile != otherTile && tile.OverlapsWith(otherTile))).ToArray();
for(int i=0; i < MaxMushrooms; i++)
{
   ...
}
Tile[] tiles = Display.Children.OfType<Tile>().Where(r => r.Type == TileType.GrassA).ToArray();
for(int i = 0; i < MaxTrees; i++)
{
   ...
}
tiles = tiles.Where(tile =>!tiles.Any(otherTile => tile != otherTile && tile.OverlapsWith(otherTile))).ToArray();
for(int i=0; i < MaxMushrooms; i++)
{
   ...
}
Also in the MaxTrees and MaxMushrooms loops, you perform a check on the type of the bgTile every loop, but the type can never be DestructableTile because you havent added any of type GrassA at any point before that. I think you can remove that check but double check before committing to that.
104 replies