search

Regalis11 / scpcb

SCP - Containment Breach
367 stars 106 forks source link

Fixed some PreventRoomOverlap issues #260

Open Sooslick opened 1 year ago

Sooslick commented 1 year ago

Fixed some issues and false-negatives in PreventRoomOverlap function: 1) Added swapping min/max in CalculateRoomExtents function before adjusting size (causing room extents stretch instead of shrink); 2) Moved CalculateRoomExtents from CreateRoom function to CreateMap to calculate initial extents correctly; 3) Fixed misleading "replaced room successfully" message for the case where rooms hadn't been replaced at all.

ChronoQuote commented 1 year ago

+1

I made the same CalculateRoomExtents fix in #210 but I never noticed that the room extents were being calculated before the rooms were rotated into place.

I'm also working on improving the PreventRoomOverlap function.

Jabka666 commented 1 year ago

What about CreateRoom functions in Save.bb? You also should call CalculateRoomExtents, shouldn't you?

ChronoQuote commented 1 year ago

What about CreateRoom functions in Save.bb? You also should call CalculateRoomExtents, shouldn't you?

Room extents are only used by CheckRoomOverlap and PreventRoomOverlap during map generation so they're not needed afterwards.

Sooslick commented 1 year ago

I made the same CalculateRoomExtents fix

@ChronoQuote Should I remove the same changes as in #210 and change target branch to yours?

ChronoQuote commented 1 year ago

Probably I should add disableoverlapcheck=true for checkpoints in rooms.ini instead

No, there is a difference between the rooms that are hard-coded to be ignored by PreventRoomOverlap and the rooms that have disableoverlapcheck=true. The rooms that are hard-coded to be ignored (checkpoint1, checkpoint2, and start) won't be moved when PreventRoomOverlap gets run on them, but they will still be checked for overlap when PreventRoomOverlap runs on other rooms. On the other hand, any room with disableoverlapcheck=true will neither be moved when PreventRoomOverlap gets run on them nor be checked for overlap when PreventRoomOverlap runs on other rooms. The only rooms with disableoverlapcheck=true are those with geometry above or below them like room049.

So I agree with Jabka that the room extents should be calculated for the checkpoint rooms. I think you can just move the CalculateRoomExtents call on line 7559 a couple lines down below the EndIf.

@ChronoQuote Should I remove the same changes as in #210 and change target branch to yours?

Sure, if you want to do that I would merge it. But maybe that would cause #210 to include too many changes... perhaps it would be better if we moved my two PreventRoomOverlap commits from #210 over to this branch, so that this branch is for room overlap while my branch is for map generation? But I'm not sure how to do that 😅

Sooslick commented 1 year ago

I agree. I realised checkpoints problem right after sending comment, so I decided to delete it immediately (but too late I guess XD)

perhaps it would be better if we moved my two PreventRoomOverlap commits

I will try to cherry-pick your commits to this branch then