Skip to content

#23 Rock/Tree tiles#41

Open
julienpezzi wants to merge 7 commits intoptal:mainfrom
julienpezzi:main
Open

#23 Rock/Tree tiles#41
julienpezzi wants to merge 7 commits intoptal:mainfrom
julienpezzi:main

Conversation

@julienpezzi
Copy link
Contributor

  • Champions can't shoot through an obstacle
  • Champions can pass next to obstacles
  • Tiles for trees and rocks

@ptal
Copy link
Owner

ptal commented May 18, 2021

Good job!
There are some improvements to be made before merging though.

The code that traverses the battlefield in order to check if no obstacle is there should be partly extracted in the new BattlefieldTraversal class.
If you can implement a method such as visitAllCellTo(int fromX, int fromY, int toX, int toY, TileVisitor visitor), it would be better.
This method take two coordinates and visit each cell between these coordinates.

Once you did that, you can use this method in attacker to check for the obstacle, e.g., (code untested)

public boolean noObstacles(int toX, int toY, Battlefield battlefield) {
  BattlefieldTraversal traversal = new BattlefieldTraversal(battlefield);
  boolean[] hasObstacle = {false};
  traversal.visitAllCellTo(x(), y(), toX, toY, new TileVisitor() {
     public void visitRock(...) { hasObstacle[0] = true; }
     // .. ..
  });
  return hasObstacle[0];
}

@julienpezzi
Copy link
Contributor Author

julienpezzi commented May 18, 2021

If you can implement a method such as visitAllCellTo(int fromX, int fromY, int toX, int toY, TileVisitor visitor), it would be better.

I thought about doing such a method but the problem is that it depends on the trajectory of the attack and I got some errors. Like for example I had obstacles which were between the champion of coordinates(fromY, fromX) and the target of coordinates(toX, toY) but which wasn't a problem for the attack. And since there were obstacles between them, it stopped the champion from shooting.

I propose to put the noObstacles class in the BattlefieldTraversal class since the towers may be not able to shoot (maybe if the champion is close enough to a tower and hidden behind a rock, the tower can't shoot him)

Copy link
Owner

@ptal ptal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You said: " I had obstacles which were between the champion of coordinates(fromY, fromX) and the target of coordinates(toX, toY) but which wasn't a problem for the attack."

Why obstacles are not a problem for the attack?

if(x < toX && y < toY) {
for(int i = x; i < toX; i++) {
for(int j = y; j < toY; j++) {
if(!emptyTile) {
Copy link
Owner

@ptal ptal May 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

emptyTile is never modified in the loop, so the loops are not useful? Same comment for the rest.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant that according the position of the champion, the tiles to check would be different and some obstacles which are not on the way of the shoot could affect the truth value. That's why I created these 3 different methods. I don't know if it's clearer like that, it's hard to explain.

Copy link
Owner

@ptal ptal May 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I meant is that since emptyTile is never modified in the loops, the following two codes are equivalent:

Code 1:

for(int i = x; i < toX; i++) {
  for(int j = y; j < toY; j++) {
     if(!emptyTile) {
       return false;

and Code 2:

if(!emptyTile) {
  return false;
}

You never use i or j in the body of the loop.

Copy link
Contributor Author

@julienpezzi julienpezzi May 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that part is a mistake which I am currently fixing !
I was answering to "Why obstacles are not a problem for the attack?"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected the methods but there is a problem with the horizontal check which is really weird since it's basically the same method as the vertical one which is working well.
Otherwise everything does work !

Copy link
Owner

@ptal ptal May 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could actually have the same code for diagonal / vertical / horizontal checks. Please look how it is done in visitAdjacent. By using an array of directions, it will make the code much less verbose. For a mathematician, generalizing shouldn't be a problem ;-)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, you will need to pull from the main repository (see lab 5 instructions)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahah ! I will try to do this today :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants