search

tsoule88 / evolvedTD

Evolutionary tower defense game from the University of Idaho
GNU General Public License v3.0
4 stars 2 forks source link

Reproduction #153

Closed higles closed 9 years ago

higles commented 9 years ago

Creatures spawn 2 gametes upon death:

Gamete mating is random:

andyleejordan commented 9 years ago

I would not merge this until the above issues are sorted out by @higles.

doug-graham commented 9 years ago

Certainly

higles commented 9 years ago

There actually are no issues. The code runs fine. I will answer your questions as soon as I get in front of my computer On Apr 6, 2015 5:57 PM, "tanimoti" notifications@github.com wrote:

Certainly

— Reply to this email directly or view it on GitHub https://github.com/tsoule88/evolvedTD/pull/153#issuecomment-90299648.

andyleejordan commented 9 years ago

@higles "The code runs fine." != "There actually are no issues."

higles commented 9 years ago

OK here we go.... This is gonna be long and snarky because you guys are posing these questions without sufficiently looking at the code/comments.

1) We only grab chromosome 0 from the creature instead of both 0 and 1 because each gamete only gets 1 chromosome. Then when the 2 gametes get matched, each of their chromosomes combine to become the new creature's genome. @andschwa this was actually you're idea when we were working on mutation and grabbing gametes, you said just grab the 0th from that function.

2) The variance is a variance in population size. Since randomly selecting gametes for mating we'd have to either only have min pop size or mate until all gametes are mated (possibly extreme pop sizes). I placed a variance so we can have anywhere from min_pop_size to (min_pop_size + variance) creatures in the new generation.

3) We shouldn't run out of gametes. Terry and Barry made me put in the 2 gametes on death of a creature so that we'd always have enough gametes. I left this here because it's a nice safety check, but should never get used till we institute speciation again or add success chance. Terry and Barry do not want extinction to happen.

4) The *= miss was on oversight of mine from copying and pasting parts of old code for the new collision detection. But honestly it's still correct so not sure why it's brought up.

5) The while loop actually does make sense if you take a small look at it. The 2 functions that are called both return booleans and the functions themselves do change the pos till they find a pos that doesn't collide with a collidable object (creature / rock). The for loops inside the function continue to randomly change the x and y until a no collision has occured for that new creature and then that position becomes it's new starting position. These functions are called inside the "seemingly empty" while loop because it looks clean and allows for easy entry of further collision correction functions.

In short for 5. None of this is an infinite loop ever and has never acted as such or even slowed down my system for spawning a new wave. So there is no reason to believe there is an error in this.

6) ChildrenBred could be deleted if we want to. I believe @andschwa placed it in when we were finding out that speciation was broken for debugging.

7) In this case "The code runs fine" == "There are no issues". You guys just need to look a little harder before saying it looks broken.

doug-graham commented 9 years ago

I like your reasoning.

andyleejordan commented 9 years ago

Please see in-line responses.

One should not need to "look a little harder" at code that looks broken. I tested this and knew it ran fine, my point was there are issues that need to be addressed, even if the code runs fine. There are some very odd parts that did not come with explanations.

higles commented 9 years ago

By look a little harder I meant look into the function. The comment explains that we're checking for collisions and the functions themselves are pretty well commented. It's also bad practice to over comment. Which is why I keep mine straight to the point. I should not need to detail every little bit of code. All you need to do is follow the code and comments and you should easily piece it together.

When I said that it may apear broken I mean to say that anything can appear broken if it is not looked at with due diligence. Ex. Just cause I look at a watch for less than a second and no hands move doesn't mean it doesn't tick On Apr 6, 2015 7:17 PM, "Andrew Schwartzmeyer" notifications@github.com wrote:

Please see in-line responses.

One should not need to "look a little harder" at code that looks broken. I tested this and knew it ran fine, my point was there are issues that need to be addressed, even if the code runs fine. There are some very odd parts that did not come with explanations.

— Reply to this email directly or view it on GitHub https://github.com/tsoule88/evolvedTD/pull/153#issuecomment-90317281.

higles commented 9 years ago

as far as the 2 gametes goes. I do see this now. I thought we were looking at a different part of the code for when a creature normally spawns a gamete. I see now this is when the creature dies and I agree with the change. I will change this.

tsoule88 commented 9 years ago

Careful (to both sides) no flame wars please ☺ Self-explanatory and well commented code is always good. What counts as self-explanatory and well documented code is a little harder to define (actually its provable impossible to define, but this email is too small for the proof).

From: HIggles [mailto:notifications@github.com] Sent: Monday, April 06, 2015 7:46 PM To: tsoule88/evolvedTD Subject: [BULK] Re: [evolvedTD] Reproduction (#153)

By look a little harder I meant look into the function. The comment explains that we're checking for collisions and the functions themselves are pretty well commented. It's also bad practice to over comment. Which is why I keep mine straight to the point. I should not need to detail every little bit of code. All you need to do is follow the code and comments and you should easily piece it together.

When I said that it may apear broken I mean to say that anything can appear broken if it is not looked at with due diligence. Ex. Just cause I look at a watch for less than a second and no hands move doesn't mean it doesn't tick On Apr 6, 2015 7:17 PM, "Andrew Schwartzmeyer" notifications@github.com wrote:

Please see in-line responses.

One should not need to "look a little harder" at code that looks broken. I tested this and knew it ran fine, my point was there are issues that need to be addressed, even if the code runs fine. There are some very odd parts that did not come with explanations.

— Reply to this email directly or view it on GitHub https://github.com/tsoule88/evolvedTD/pull/153#issuecomment-90317281.

— Reply to this email directly or view it on GitHubhttps://github.com/tsoule88/evolvedTD/pull/153#issuecomment-90323631.

BIOEvolveTD commented 9 years ago

?I've been using the reproduction branch, and I think there is a major memory leak or something. game slows down bad after a few generations.

From: HIggles notifications@github.com Sent: Monday, April 6, 2015 8:01 PM To: tsoule88/evolvedTD Subject: Re: [evolvedTD] Reproduction (#153)

as far as the 2 gametes goes. I do see this now. I thought we were looking at a different part of the code for when a creature normally spawns a gamete. I see now this is when the creature dies and I agree with the change. I will change this.

Reply to this email directly or view it on GitHubhttps://github.com/tsoule88/evolvedTD/pull/153#issuecomment-90329864.

higles commented 9 years ago

I took a look at this and am doing some forced garbage collection. But I don't see any difference from the changes I made. I tried running master and noticed a similar slow down.

My hypothesis is that this is maybe from data collection since the more rounds we run the larger the data that we're storing in the tables within the game. I think I'm going to have to look at a way to delete all the data after writing it into a file and when I want to update the data, read it in, update, write to file, then delete again.

higles commented 9 years ago

It could also be a number of other things.

BIOEvolveTD commented 9 years ago

?Yup. I am on the master at gen 32 and it is running painfully slow.

From: HIggles notifications@github.com Sent: Tuesday, April 7, 2015 10:33 AM To: tsoule88/evolvedTD Cc: Robison, Barrie (brobison@uidaho.edu) Subject: [BULK] Re: [evolvedTD] Reproduction (#153)

I took a look at this and am doing some forced garbage collection. But I don't see any difference from the changes I made. I tried running master and noticed a similar slow down.

My hypothesis is that this is maybe from data collection since the more rounds we run the larger the data that we're storing in the tables within the game. I think I'm going to have to look at a way to delete all the data after writing it into a file and when I want to update the data, read it in, update, write to file, then delete again.

Reply to this email directly or view it on GitHubhttps://github.com/tsoule88/evolvedTD/pull/153#issuecomment-90660681.

BIOEvolveTD commented 9 years ago

?I don't think it is the appendages. I've increased the mutation deviation a lot to get more interesting diversity in early generations. We need a mode where the gun being turned off doesn't auto pause to test the bullet deletion hypothesis.

From: HIggles notifications@github.com Sent: Tuesday, April 7, 2015 10:39 AM To: tsoule88/evolvedTD Cc: Robison, Barrie (brobison@uidaho.edu) Subject: [BULK] Re: [evolvedTD] Reproduction (#153)

It could also be a number of other things.

Reply to this email directly or view it on GitHubhttps://github.com/tsoule88/evolvedTD/pull/153#issuecomment-90664403.

higles commented 9 years ago

Yes and I do notice that it slows down a bit earlier in the reproduction branch. This is probably because there's always at least 35 creatures per generation compared to master which still allows for extinction. This is why I think it's a data collection issue. I will start working on this, however we will most likely start seeing a pause at the end of each round due to this fix.

Other than that, I can't find any changes in the reproduction branch that actually causes memory leakage

andyleejordan commented 9 years ago

@higles thank you for addressing those comments! This PR is coming along nicely.

However, for this piece of code:

// Check coordinates for other creatures or rocks spawned in this tile.
while( checkForCreature(pos, generation) || checkForRock(pos, rocks)){};

It sure seems to me that the intent is more like this:

// Search for the closest valid tile without an obstruction (rock, creature, or otherwise)
Vec2 pos = getValidPos(pos, generation, rocks, ...);

Since finding a valid position on which to place the gamete is a necessary step for the program to continue, it does not make sense for it to be done as part of a two-sided conditional check in an empty while loop. As-is, there is a lot of indirection happening that will make this very difficult to maintain. I highly recommend this be refactored for more clarity.

andyleejordan commented 9 years ago

Also, per @higles comment in a36e67e:

added a few comments for people that are struggling to follow function calls

I'm sure we all appreciate code being made easier to follow and maintain :neckbeard:

higles commented 9 years ago

@andschwa // Search for the closest valid tile without an obstruction (rock, creature, or otherwise) Vec2 pos = getValidPos(pos, generation, rocks, ...);

Is something I had tried to do first. However, there are issues with it. You have different sized lists which would require you to check one list and then the other. However if a collision is found at any time you need to restart your checks through the lists till you never get a collision. This is what the while loop does.

And splitting into separate check functions for the different types is necessary due to the arrayLists being of different types. Splitting these up allows me to get the pos info for the creature/rock object and then send it to the same collision checker function.

Splitting all this up also makes it much simpler for adding in collision detection for other objects later. All that would be required is copy/paste the creature or rock checker function and change the arrayList type for the new object type. Then you just add a call to the while condition. Very easy

higles commented 9 years ago

I have determined that the slow down is due to the bullets and not data collection or appendages.

To test this I ran master with the gun active. After 10 rounds there was a decent amount of slowdown and even some render pausing.

I then ran the master with the gun inactive. After 13 rounds I turned the gun back on and it fired just as fast as if it were firing in the first round.

Both tests had data collection turned off so I knew that that was not a factor at all. So now we know that there is something with the gun or bullets. I'm guessing it's the bullet box2d objects not all getting deleted.

doug-graham commented 9 years ago

I can fix this right now in the tower-specific-upgrades branch. It'll be done and merged within an hour.

higles commented 9 years ago

Ok. Well keep in mind this is still a hypothesis. Please retest my test before making such changes. Maybe also try other tests.

However I'm pretty sure this is the problem.