-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Spawn and Free Cell Search Behavior #8324
Conversation
- When searching for a map-wide free cell, the tiles 15 cells from the edge are no longer considered * Added a configuration to change the edge size to any value between 4 and 40 - When searching for a free cell, the tiles 4-5 cells from the edge are now considered invalid and trigger a retry - Searching for a free cell now defaults to 50 tries, but if the "no spawn on player" option is active, those failed attempts are not counted towards the limit anymore - When a monster spawns in a defined area there will now be 9 attempts to spawn it on a valid cell within the area; if all 9 attempts fail, there will now be 50 tries to spawn it map-wide before it gives up - When a monster has fixed spawn coordinates, but those coordinates are a wall, it will now spawn in a random location map-wide instead - Fixes #8300
- Reduced number of area tries to 8 - If all tries fail and the center cell is not a wall, the monster will spawn there
Other code needs to be adjusted individually Co-authored-by: Lemongrass3110 <[email protected]>
if (!map_search_freecell(&md->bl, -1, &md->bl.x, &md->bl.y, md->spawn->xs-1, md->spawn->ys-1, battle_config.no_spawn_on_player?4:0, 8)) | ||
{ | ||
// If area search failed and center cell not reachable, try to spawn the monster anywhere on the map (50 tries) | ||
if (!map_getcell(md->bl.m, md->bl.x, md->bl.y, CELL_CHKREACH) && !map_search_freecell(&md->bl, -1, &md->bl.x, &md->bl.y, -1, -1, battle_config.no_spawn_on_player?4:0)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note:
Here this is not the (original) center cell anymore. It was already adjusted by the call to map_search_freecell.
Therefore the first part of the if is useless.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not useless. If you are implying it cannot currently be on an unwalkable cell, then the whole if() would be useless and not just the first part.
The first part of the if would only be useless if we are guaranteed to currently be on an unwalkable cell which is definitely not guaranteed.
Basically if the official spawn search cell algorithm fails and then current cell is walkable, then it should always spawn on that cell (even if it's the cell that was found by the first call of map_search_freecell), but if the cell is unwalkable, then the monster spawns in a random area of the map.
@@ -1754,10 +1756,11 @@ int map_search_freecell(struct block_list *src, int16 m, int16 *x,int16 *y, int1 | |||
if (spawn >= 100) return 0; //Limit of retries reached. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (spawn >= 100) return 0; //Limit of retries reached. |
The battle config is already limited [0,100]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will break the feature. It's different because if you set it to 100 the monster should not spawn at all, whereas if the value is lower than 100, it should spawn anyway.
md->bl.x = md->centerX; | ||
md->bl.y = md->centerY; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
md->bl.x = md->centerX; | |
md->bl.y = md->centerY; | |
if( battle_config.randomize_center_cell ){ | |
md->bl.x = md->centerX; | |
md->bl.y = md->centerY; | |
}else{ | |
md->bl.x = md->spawn->x; | |
md->bl.y = md->spawn->y; | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed, md->centerX is set to md->spawn->x during initialization of the spawnset.
if ((!md->spawn->state.boss || (md->bl.x == 0 && md->bl.y == 0) || md->spawn->xs != 1 || md->spawn->ys != 1) | ||
&& (md->spawn->xs + md->spawn->ys < 1 || !rnd_chance(1, md->spawn->xs * md->spawn->ys) || !map_getcell(md->bl.m, md->bl.x, md->bl.y, CELL_CHKREACH))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the complexity of this condition I would say that it may be better to put this into a function and make those conditions return early.
Otherwise you could create a pseudo scope, to be able to break out of it as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not really that complex and the comment explains it too. Also too much structural change so close before pushing would just be more risky than not touching it anymore.
// Determine center cell for each mob in the spawn line | ||
if (battle_config.randomize_center_cell) { | ||
if (mob->xs > 1) | ||
md->centerX = rnd_value(mob->x - mob->xs + 1, mob->x + mob->xs - 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
md->centerX = rnd_value(mob->x - mob->xs + 1, mob->x + mob->xs - 1); | |
md->centerX = rnd_value(mob->x - mob->xs + 1, mob->x + mob->xs - 1); | |
else | |
md->centerX = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's just leave centerX as initialized, then we also don't need the other changes!
if (mob->xs > 1) | ||
md->centerX = rnd_value(mob->x - mob->xs + 1, mob->x + mob->xs - 1); | ||
if (mob->ys > 1) | ||
md->centerY = rnd_value(mob->y - mob->ys + 1, mob->y + mob->ys - 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
md->centerY = rnd_value(mob->y - mob->ys + 1, mob->y + mob->ys - 1); | |
md->centerY = rnd_value(mob->y - mob->ys + 1, mob->y + mob->ys - 1); | |
else | |
md->centerY = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above
md->centerX = rnd_value(mob->x - mob->xs + 1, mob->x + mob->xs - 1); | ||
if (mob->ys > 1) | ||
md->centerY = rnd_value(mob->y - mob->ys + 1, mob->y + mob->ys - 1); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | |
}else{ | |
md->centerX = 0; | |
md->centerY = 0; | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above
Description of Pull Request: