16

You have common code, which moreover has applications beyond this one, so should you not pull it out into a function? Then you can reduce overlap to def overlap(r1, r2): '''Overlapping rectangles overlap both horizontally & vertically ''' return range_overlap(r1.left, r1.right, r2.left, r2.right) and range_overlap(r1.bottom, r1.top, r2....


14

Collision detection You can calculate the collision much easier with the Rectangle structure. Use the Contains method if you are interested in single points that Determines if the specified point is contained within this Rectangle structure. var pictureRect = new Rectangle( pictureBox.Location.X, pictureBox.Location.Y, pictureBox.Width, ...


13

In computer geometry, always use vectors if possible! Code gets more complicated if you try to work with Cartesian co-ordinates \$ (x, y) \$ or with line equations \$ y = mx + b \$. Here, for example, you have special cases for horizontal lines, \$ m = 0 \$, and vertical lines, \$ m = \infty \$. So let's try to program this, sticking to vectors throughout. ...


12

crossImage1 = pygame.image.load('cross.png') crossImage2 = pygame.image.load('cross.png') crossImage3 = pygame.image.load('cross.png') crossImage4 = pygame.image.load('cross.png') crossImage5 = pygame.image.load('cross.png') crossImage6 = pygame.image.load('cross.png') crossImage7 = pygame.image.load('cross.png') crossImage8 = pygame.image.load('cross.png') ...


11

if (this.velocity > dampening) { this.velocity -= dampening; } else { this.velocity = 0; } this.velocity = -this.velocity; Seems like you could use Math functions for this: this.velocity = Math.min(dampening - this.velocity, 0); That will simplify it somewhat. Logic behind it: if (this.velocity > dampening) { this.velocity -= dampening;...


10

IMHO this is perfectly fine. if (!isPlayer(bodyA) || !isPlayer(bodyB)) { return; } This is an early return, saving you one indentation level. if (save(bodyB)) return; markForDestroy(bodyB); This is not as good. Here, I'd go for if (!save(bodyB)) markForDestroy(bodyB); Note that braces should be used around single statements as well, i.e....


9

Naming s is a poor name for a boolean array. I have no idea what s stands for. How about surroundingTiles? Array ordering Additionally, the order of the tiles seems strange to me. It's ... bottom, right, top, left, bottom right, top right, top left, bottom left. Without the image, I don't think I'd have gotten it. (For those wondering, it's non-diagonal, ...


8

Looking at your profiling information you spend a total of 14.5s in handle_collision() out of which 3.8s is spent in get_nearby_entities. So your culprit may actually be somewhere else (I can't tell without the rest of your source). Precalculate/Cache nearby status You are calculating the nearby entities too many times. Consider this, you have a cluster of ...


8

I would simply apply a logic transformation. Here's your original, verbatim: def overlap(r1,r2): '''Overlapping rectangles overlap both horizontally & vertically ''' hoverlaps = True voverlaps = True if (r1.left > r2.right) or (r1.right < r2.left): hoverlaps = False if (r1.top < r2.bottom) or (r1.bottom > r2....


8

Before making this more efficient I'd suggest making it more readable: in your switch statement you have break in both the true and else parts of your if statement. if(checkmove(pos.x, pos.y-2)) { pos.y -= 2; break; } else { break; } It would be much easier to read if you just move the break out of the if statement. if(checkmove(pos.x, pos.y-...


8

You should always close any file you open. You can guarantee this by using a context manager. with open("some_file_name") as f: x = int(f.readline()) This is a safe (and clean) way to guarantee that a file is closed, no matter what happens inside the with block.


7

Is it a good idea to be modifying the this object? For instance does the following pattern make sense? function Point(I) { this.x = I.x; this.y = I.y; this.z = I.z; return this; } Yes and no. Modifying this is normal in a constructor function, but what you're doing here is unusual. Something like this would be more common: function Point(x,...


7

You're not using the C# naming convention: class ClassNamesShouldBePascalCase { const CONTANTS_ARE_IN_ALL_CAPS; private _membersShouldStartWithAUnderScoreAndBeCamelCase; //_camelCase public PropertiesShouldBePascalCase {get; set;} //PascalCase public void PublicMethodsShouldBePascalCase { object ...


7

Just a quick answer. You are calculating s.Width / 2 for each Picturebox two times. Just calculate it once, store the result in a variable and use the variable. You are calculating player.Width / 2 twice for each Picturebox which should be done outside of the loop and stored in a variable. The same applies to the calculation of player.Height/ 2. This ...


7

You are creating a new Player and Tile every time this code executes. That is likely your performance hit. Try moving the existing player rather than creating a new one in a new location. Does performance slow down further the longer the game runs? This is because your objects are piling up. For further reading I recommend looking at AABB Collision ...


6

In response to not triggering the message repeatedly, I would store the previous check for a collision so you know if a collision has started or ended. If there is a collision but your previous check didn't show a collision, then you have a new collision and you can trigger the event. However, if the previous check did show a collision, don't fire the ...


6

From a once-over, Short 1 letter variables are terrible ( except for x and y ), b, p and W are just terrible Also please spellcheck your variables: padles -> paddles You should only detect collision when the ball is going to the left and gets in the danger-zone or when it is going to the right and getting in the danger-zone. So instead of doing 8 accesses, ...


6

Use auto-properties when you can like in RigidObject and set default values in the constructor. public double Mass { get; set; } I prefer to keep my private and protected members named with prefix underscore, and my method variables just lowercase. I saw you do some of this, and some not... it is good to keep this consistent. While I'm on naming and ...


6

1. Vectors Much of this code is awkward and long-winded because points and vectors are represented by plain Python tuples. A simple operation like subtracting two points requires disassembling the points into their elements, subtracting the elements, and then reassembling the result. If the code represented points using some kind of vector data structure, ...


6

Those refactoring will help : idea is to test only upper-left point and bottom-right point, which should be ok 99.99999% of the time. (Rq : i know we could test wether the second test is in fact about the same tile, but i guess that would be slower). function checkmove(x, y, w, h) { var tileX, tileY, thisTile; tileX = Math.floor(x/20) ; tileY = ...


6

The first early return is fine. if (!isPlayer(bodyA) || !isPlayer(bodyB)) { return; } This immediately tells the programmer that both bodies must be players. The later ones less so, You can invert the condition and put the markForDestroy call in the then clause. if (!save(bodyB)) markForDestroy(bodyB); I would add some blank lines to break up ...


6

As noted in the comments, this would be better broken into methods. Not only would it make it easier to profile, it would make the code easier to understand. angle = player.getAngle(); This seems a bad name. What is an angle and why does a player have one? My best guess is that this angle represents the direction of travel. If so, why not call it ...


6

Welcome here! First of all, you have a lot of vars for images. Consider using image lists, like: CROSS_IMAGE_FILENAME = ['cross.png'] * 8 # or different list crossImages = [pygame.image.load(filename) for filename in CROSS_IMAGE_FILENAME] Camera and current position should be grouped into Points or something else, so instead of SomethingX and SomethingY ...


6

Virtual destructor As a simple rule of thumb, if a base class contains any virtual functions, its destructor should also be virtual. The primary reason for it to declare a virtual function is so a derived class can override that virtual function. If you have public derivation, you nearly always want the destructor to be virtual--without that, destroying a ...


5

I don't think there are any strong reasons towards any of the approaches (object vs values), but I will list out all the advantages/disadvantages of both ways so that you'll choose what seems better for you: Passing in an object of the structure you're describing (function Point(I)) [+] creating new instances from older instances or compatible objects (...


5

Here's the simplest rewrite I could come up with for your artifact collision detection algorithm: module Plane where type Location = (Float, Float) type Size = (Float, Float) data Artifact = Artifact { location :: Location, size :: Size } xMin (Artifact (x, y) (w, h)) = x - w / 2 xMax (Artifact (x, y) (w, h)) = x + w / 2 yMin (Artifact (x, y) (w, h)) = y -...


5

You play around with different ordering (depending on you data) it may be faster to check absolutes first, or last. if(leftTile < 0 || leftTile >= map.getWidth() || topTile < 0 ||.... can become if(leftTile < 0 || topTile < 0 || leftTile >= map.getWidth()... Also, notice that you throw away half your answers after a fast check, plenty ...


5

This line: if(level[Math.floor(x/20)][Math.floor(y/20)] == 1 || level[Math.ceil(x/20)][Math.floor(y/20)] == 1 || level[Math.floor(x/20)][Math.ceil(y/20)] == 1 || level[Math.ceil(x/20)][Math.ceil(y/20)] == 1) { is really long and makes your code difficult to read. You can refactor this to something much easier on the eyes: if(level[Math.floor(x/20)][Math....


5

You have some code duplication there for(int row = topLeft.y; row <= bottomRight.y; row++) { if(map.getType(row, topLeft.x) == Tile.SOLID) { collision = true; break; } } which should be extracted to a method like private boolean hasCollision(int start, int stop, int x){ for(int row = start; row <= stop; row++) ...


5

All GUI updates must be performed on the GUI thread. But you create new GUI elements in the main method of the BasicPlatformer, which runs on the main thread. It is not correct. You should use the invokeLater method and a Runnable(possibly with a lambda expression) instead. It looks like there is no need for multiple timers here. One timer(with the shortest ...


Only top voted, non community-wiki answers of a minimum length are eligible