0

I am trying to create a game, where a player can move around the games map pulled from a text file. It has 2 stores of the map, originalmap and playermap. originalmap is used to reference the original state of the map, so that the player can move over tiles that have a special nature. playermap is what the player sees, and the changes, such as moving and spawning the player itself happens in playermap. Below is the code where both originalmap and playermap is created. Both maps are 2d arraylists.

class map {
  //properties of the map.
  private List<List<Character>> originalmap = new ArrayList<List<Character>>();
  private List<List<Character>> playermap = new ArrayList<List<Character>>();
  //read a map file. Existence is checked when entering filepath, so no need to code any catches.
  private void createmap() {
    try {
      File mapFile = new File(this.filepath);
      Scanner textfile = new Scanner(mapFile);
      //read the entire mapfile, line by line. For every line, a mapLine converts the line into a list of characters, which is appended to the map variable.
      this.mapname = textfile.nextLine().replace("name ", "");
      String line2 = textfile.nextLine();
      this.goldreq = Integer.parseInt(Character.toString(line2.charAt(line2.length()-1)));
      while (textfile.hasNextLine()) {
        String line = textfile.nextLine();
        List<Character> mapLine = new ArrayList<Character>();
        for (int i = 0; i<line.length();i++) {
          mapLine.add(line.charAt(i));  
        }
        this.originalmap.add(mapLine); 
        this.playermap.add(mapLine);
      }
      textfile.close();
    } catch (FileNotFoundException error) {
      assert true;
    }
  }

The issue is when I try to spawn a player, it appears that the player spawns on both the originalmap and the playermap. I am at a loss as to why. Below is the code where the player is spawned.

  public void spawnPlayer() {
    Random rand = new Random();
    while (true) {        
      int spawnx = rand.nextInt(playermap.get(0).size());
      int spawny = rand.nextInt(playermap.size());
      char spawnchar = playermap.get(spawny).get(spawnx);
      if (spawnchar!= '#'||spawnchar != 'G'||spawnchar != 'B') {
        this.playergold = 0;
        this.playerposy = spawny;
        this.playerposx = spawnx;
        this.playermap.get(spawny).set(spawnx,'P');
        break;
      }
    }
  }
}

N/B: these functions are called one after another (createmap, and then spawnplayer) in the default method of the class.

I tried to print the maps, expecting to see originalmap to be unchanged and playermap to be changed. Both maps were changed, without me changing originalmap after spawning the player.

  • 1
    Does this answer your question? [Is Java "pass-by-reference" or "pass-by-value"?](https://stackoverflow.com/questions/40480/is-java-pass-by-reference-or-pass-by-value) – jhamon Dec 16 '22 at 13:58
  • 2
    You add the same ArrayList `mapLine` to bot your `originalmap` and `playermap`. Any modification you do on that list will therefor be reflected everywhere else it is used. In java objects don't get automatically cloned/copied when you pass them around. – OH GOD SPIDERS Dec 16 '22 at 13:59
  • `if (spawnchar!= '#'||spawnchar != 'G'||spawnchar != 'B') ` is always true. You probably want *and* operator `&&` instead of *or* `||`. More info [Why does non-equality check of one variable against many values always return true?](https://stackoverflow.com/q/26337003) – Pshemo Dec 16 '22 at 14:03
  • @jhamon No, that doesn't seem related. – Konrad Rudolph Dec 16 '22 at 14:03
  • `assert true;` isn't very useful. – Konrad Rudolph Dec 16 '22 at 14:03
  • @jhamon I referred to it, I don't understand it fully, but what i got it was that if i did this.playermap = originalmap, it would be a pass by reference, which i tried to avoid by updating both arraylists through mapline. – Rhian Kansara Dec 16 '22 at 14:04
  • @OHGODSPIDERS thats why im confused - i dont use mapline in spawnplayer, its unique to the createmap function. – Rhian Kansara Dec 16 '22 at 14:07
  • 1
    it's the same object in both map. When you change the line list in the first map, the one in the second map is also edited as it's the same object – jhamon Dec 16 '22 at 14:08
  • @KonradRudolph check this part `this.originalmap.add(mapLine); this.playermap.add(mapLine); ` – jhamon Dec 16 '22 at 14:12
  • @KonradRudolph the `assert true` acts as a pass statement from my understanding, is theres any other way to do this? – Rhian Kansara Dec 16 '22 at 14:14
  • Also you posted code where you say "where both originalmap and playermap is created", but no neither of these are created in this code, this is simply where they are modified, where are they created and what do they look like? – Nexevis Dec 16 '22 at 14:14
  • @jhamon I understand, but the answer "Java is pass by value" doesn't really explain what is going on: OP *doesn't think* that Java is pass by reference. The thing is that they don't *understand* pass by reference, and that isn't the focus of the linked question (at best it's explained in passing in one of the numerous answers). – Konrad Rudolph Dec 16 '22 at 14:14
  • 1
    @RhianKansara The `assert true;` statement has *no effect*. Just remove it, it does nothing. I don't know what you mean by "pass statement". At any rate, you cannot just ignore a `FileNotFoundException`. You must handle it: your application cannot proceed if the file wasn't found. – Konrad Rudolph Dec 16 '22 at 14:15
  • Also this code `char spawnchar = playermap.get(spawny).get(spawnx);` looks suspicious to me, but without knowing what `playermap` is it is hard to say what exactly is wrong. But you are somehow calling `.get` on a `char`? Or else how is the second `get` returning a `char`. I guess maybe the first `get` is returning another map? – Nexevis Dec 16 '22 at 14:19
  • @jhamon its the same data in createmap, but in spawnplayer i exclusively use playermap - unless im mistaken and theyre linked somehow? – Rhian Kansara Dec 16 '22 at 14:26
  • @Nexevis `class map { //properties of the map. private List> originalmap = new ArrayList>(); private List> playermap = new ArrayList>();` - i believe that answers the other questions as well – Rhian Kansara Dec 16 '22 at 14:27
  • @Rhian Please edit your question with the new info, as it might help other people as well. Also those are not maps, those are lists, definitely confusingly named. I guess it doesn't mean map in the programming sense, but map as in a literal topographical map – Nexevis Dec 16 '22 at 14:28
  • of course they are linked. `this.playermap` from createmap, is still the same `this.playerrmap` from spawnplayer. Same for `this.this.originalmap`. As both map have the exact same objects inside in createmap, it's still the same content in spawnplayer – jhamon Dec 16 '22 at 14:29
  • @KonradRudolph the files existence is checked when the user enters a filepath - which is why i just ignore it – Rhian Kansara Dec 16 '22 at 14:30
  • 1
    @Rhian Kansara What if the file is deleted in between the code executing? It is bad practice to swallow exceptions without any logging – Nexevis Dec 16 '22 at 14:32
  • @jhamon how would i go about fixing that? – Rhian Kansara Dec 16 '22 at 14:34
  • instead of using a single list `mapLine`, simply create a 2nd one `mapPlayerLine` – jhamon Dec 16 '22 at 14:36

1 Answers1

1

Here are the problem statements that are related:

In createMap:

this.originalmap.add(mapLine); 
this.playermap.add(mapLine);

In spawnPlayer:

this.playermap.get(spawny).set(spawnx,'P');

When you add the same mapLine to each List, you are not adding a duplicate of the values you are adding a reference to the same List.

The next problem statement will then grab the List from playermap and change the value, which will of course affect both playermap and originalmap because they are referencing the same exact lists.

To fix this, you simply need to create two separate List, playerMapLine and originalMapLine:

private void createmap() {
    try {
      File mapFile = new File(this.filepath);
      Scanner textfile = new Scanner(mapFile);
      //read the entire mapfile, line by line. For every line, a mapLine converts the line into a list of characters, which is appended to the map variable.
      this.mapname = textfile.nextLine().replace("name ", "");
      String line2 = textfile.nextLine();
      this.goldreq = Integer.parseInt(Character.toString(line2.charAt(line2.length()-1)));
      while (textfile.hasNextLine()) {
        String line = textfile.nextLine();
        List<Character> playerMapLine = new ArrayList<>();
        List<Character> originalMapLine = new ArrayList<>();

        for (int i = 0; i<line.length();i++) {
            playerMapLine.add(line.charAt(i));  
            originalMapLine.add(line.charAt(i));  
        }
        this.playermap.add(playerMapLine);
        this.originalmap.add(originalMapLine); 
      }
      textfile.close();
    } catch (FileNotFoundException error) {
      error.printStackTrace();
    }

Also, please do not swallow an Exception and print the statement out with error.printStackTrace(), or log it properly so when an error happens, it is clear.

Nexevis
  • 4,647
  • 3
  • 13
  • 22