Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

Are you sure you really should use else if here? I think that you just need to use if. Because another rect can collide with both bottom, bottom-left, and bottom-right at the same time.

if (BOTTOM.IntersectsWith(rects.rect))
{
 bBOTTOM = false;
}
else if (BOTTOM_LEFT.IntersectsWith(rects.rect))
{
 bBOTTOM_LEFT = false;
}
else if (BOTTOM_RIGHT.IntersectsWith(rects.rect))
{
 bBOTTOM_RIGHT = false;
}

Now, I believe however that there is a more efficient method to do this. I'd like to call it...

#More math, less code

More math, less code

Instead of using code and testing collisions, use more math and methods that are available on the Rectangle objects and do calculations with them.

Pseudocode following for how I believe you can do this. I don't have time to work out the exact code in C# (partially because I'm no C# expert). I am not sure how much more efficient it will be, and I'm not entirely sure if it will work, but I believe it should be correct and work:

The idea here is to calculate the union of all the rectangle intersections you will get by moving.

MovingRect = the rectangle you want to move
MovingRect.y++
intersection = empty rectangle
for each (`rect` in all other rectangles) {
 if (rect intersects with MovingRect) {
 thisIntersection = calculate how much / where it intersects: See link below
 intersection = union of intersection with thisIntersection: See link below
 if the width of the intersection is two or more, then you can't move at all.
 }
}
check the size and position of the `intersection` rectangle:
 if it's size is nonexistent (i.e. empty rectangle), then you are free to move down one step
 else if it's width is 2 or more, then you are unable to move at all
 else if it's position is in the lower-left area of your rectangle, move right one step
 else if it's position is in the lower-right area of your rectangle, move left one step
 else throw an AssertionError because something is apparently wrong with this logic.

Links:

For the calculation of the intersection area

For the union of rectangles

Are you sure you really should use else if here? I think that you just need to use if. Because another rect can collide with both bottom, bottom-left, and bottom-right at the same time.

if (BOTTOM.IntersectsWith(rects.rect))
{
 bBOTTOM = false;
}
else if (BOTTOM_LEFT.IntersectsWith(rects.rect))
{
 bBOTTOM_LEFT = false;
}
else if (BOTTOM_RIGHT.IntersectsWith(rects.rect))
{
 bBOTTOM_RIGHT = false;
}

Now, I believe however that there is a more efficient method to do this. I'd like to call it...

#More math, less code

Instead of using code and testing collisions, use more math and methods that are available on the Rectangle objects and do calculations with them.

Pseudocode following for how I believe you can do this. I don't have time to work out the exact code in C# (partially because I'm no C# expert). I am not sure how much more efficient it will be, and I'm not entirely sure if it will work, but I believe it should be correct and work:

The idea here is to calculate the union of all the rectangle intersections you will get by moving.

MovingRect = the rectangle you want to move
MovingRect.y++
intersection = empty rectangle
for each (`rect` in all other rectangles) {
 if (rect intersects with MovingRect) {
 thisIntersection = calculate how much / where it intersects: See link below
 intersection = union of intersection with thisIntersection: See link below
 if the width of the intersection is two or more, then you can't move at all.
 }
}
check the size and position of the `intersection` rectangle:
 if it's size is nonexistent (i.e. empty rectangle), then you are free to move down one step
 else if it's width is 2 or more, then you are unable to move at all
 else if it's position is in the lower-left area of your rectangle, move right one step
 else if it's position is in the lower-right area of your rectangle, move left one step
 else throw an AssertionError because something is apparently wrong with this logic.

Links:

For the calculation of the intersection area

For the union of rectangles

Are you sure you really should use else if here? I think that you just need to use if. Because another rect can collide with both bottom, bottom-left, and bottom-right at the same time.

if (BOTTOM.IntersectsWith(rects.rect))
{
 bBOTTOM = false;
}
else if (BOTTOM_LEFT.IntersectsWith(rects.rect))
{
 bBOTTOM_LEFT = false;
}
else if (BOTTOM_RIGHT.IntersectsWith(rects.rect))
{
 bBOTTOM_RIGHT = false;
}

Now, I believe however that there is a more efficient method to do this. I'd like to call it...

More math, less code

Instead of using code and testing collisions, use more math and methods that are available on the Rectangle objects and do calculations with them.

Pseudocode following for how I believe you can do this. I don't have time to work out the exact code in C# (partially because I'm no C# expert). I am not sure how much more efficient it will be, and I'm not entirely sure if it will work, but I believe it should be correct and work:

The idea here is to calculate the union of all the rectangle intersections you will get by moving.

MovingRect = the rectangle you want to move
MovingRect.y++
intersection = empty rectangle
for each (`rect` in all other rectangles) {
 if (rect intersects with MovingRect) {
 thisIntersection = calculate how much / where it intersects: See link below
 intersection = union of intersection with thisIntersection: See link below
 if the width of the intersection is two or more, then you can't move at all.
 }
}
check the size and position of the `intersection` rectangle:
 if it's size is nonexistent (i.e. empty rectangle), then you are free to move down one step
 else if it's width is 2 or more, then you are unable to move at all
 else if it's position is in the lower-left area of your rectangle, move right one step
 else if it's position is in the lower-right area of your rectangle, move left one step
 else throw an AssertionError because something is apparently wrong with this logic.

Links:

For the calculation of the intersection area

For the union of rectangles

Source Link
Simon Forsberg
  • 59.7k
  • 9
  • 157
  • 311

Are you sure you really should use else if here? I think that you just need to use if. Because another rect can collide with both bottom, bottom-left, and bottom-right at the same time.

if (BOTTOM.IntersectsWith(rects.rect))
{
 bBOTTOM = false;
}
else if (BOTTOM_LEFT.IntersectsWith(rects.rect))
{
 bBOTTOM_LEFT = false;
}
else if (BOTTOM_RIGHT.IntersectsWith(rects.rect))
{
 bBOTTOM_RIGHT = false;
}

Now, I believe however that there is a more efficient method to do this. I'd like to call it...

#More math, less code

Instead of using code and testing collisions, use more math and methods that are available on the Rectangle objects and do calculations with them.

Pseudocode following for how I believe you can do this. I don't have time to work out the exact code in C# (partially because I'm no C# expert). I am not sure how much more efficient it will be, and I'm not entirely sure if it will work, but I believe it should be correct and work:

The idea here is to calculate the union of all the rectangle intersections you will get by moving.

MovingRect = the rectangle you want to move
MovingRect.y++
intersection = empty rectangle
for each (`rect` in all other rectangles) {
 if (rect intersects with MovingRect) {
 thisIntersection = calculate how much / where it intersects: See link below
 intersection = union of intersection with thisIntersection: See link below
 if the width of the intersection is two or more, then you can't move at all.
 }
}
check the size and position of the `intersection` rectangle:
 if it's size is nonexistent (i.e. empty rectangle), then you are free to move down one step
 else if it's width is 2 or more, then you are unable to move at all
 else if it's position is in the lower-left area of your rectangle, move right one step
 else if it's position is in the lower-right area of your rectangle, move left one step
 else throw an AssertionError because something is apparently wrong with this logic.

Links:

For the calculation of the intersection area

For the union of rectangles

lang-cs

AltStyle によって変換されたページ (->オリジナル) /