0

This is an example from a Pokemon-like game. I am constructing an Object, and inside it i am trying to make a new Object "en" and "to", that is two different attacks. The problem is that when i try to edit something in either of the attack Objects ("en" and "two"), the change happens to every Pokemon with the same name. This doesn't happen with the "health", so i think that the this.en = new Object; is the problem.

This is the code for constructing the Pokemon

function Fakemon(_navn, _type, _attackPower, _src,
    _1Navn, _1Force, _1Antall_, _2Navn, _2Force, _2Antall) {
    this.navn = _navn;
    this.type = _type;
    this.attackPower = _attackPower;
    this.src = _src;

    this.en = new Object;
    this.en.navn = _1Navn;
    this.en.force = _1Force;
    this.en.antall = _1Antall_;

    this.to = new Object;
    this.to.navn = _2Navn;
    this.to.force = _2Force;
    this.to.antall = _2Antall;

    this.health = 1000;

    console.log(this.en);
    this.pushFakemon = function() {
        fakemonSamling.push(this);
    }
    this.pushFakemon();
}

const fakemon1 = new Fakemon("BatCat", "Flying", [10, 50], ["batFront.png", "batBack.png"], "Blood Suck", [25, 38, 60], 10, "Wing Slap", [10, 17, 25], 20);
const fakemon2 = new Fakemon("Muffin Time", "Normal", [15, 45], ["cupcakeFront.png", "cupcakeBack.png"], "Frosting cover", [10, 17, 25], 20, "Cake stomp", [40, 50, 60], 5);

This is the Code for putting three Pokemon's to each player

for (let i = 0; i < 3; i++) {
        var temp1 = new Object;
        player1.push(Object.assign(temp1, randomFakemon()));
        var temp2 = new Object;
        player2.push(Object.assign(temp2, randomFakemon()));
    }
Peter Seliger
  • 11,747
  • 3
  • 28
  • 37
  • 2
    can you share the code you use when you "try to edit something in either of the attack Objects" ? – gui3 May 16 '22 at 11:40
  • 1
    No, the problem is not in your constructor. Rather it's that `randomFakemon()` can return the *same* fakemon multiple times, instead of creating a new one every time. – Bergi May 16 '22 at 12:13
  • Btw, [using `new Object` is rather unidiomatic, prefer object literals](https://stackoverflow.com/q/251402/1048572) – Bergi May 16 '22 at 12:16
  • Another thought ... in case the OP can not express the intention of a function, especially constructor function, with maybe up to 3 or 4 parameters the OP might think about using 1 to 3 config objects as parameters. For the OP's use case even more so as all the arguments just get assigned as public properties ... think about `Object.assign(this, configA, { en: ...configB, to: ...configC });` – Peter Seliger May 16 '22 at 14:46
  • ... code review part 2 ... the implementation of `pushFakemon` method (either own or prototypal) is dispensable. Instead push the newly created instance directly. – Peter Seliger May 16 '22 at 14:55
  • 3/4 ... also, and _Bergi_ already touched the subject, in case `randomFakemon()` returns randomly **references** from the fakemon collection called `fakemonSamling` does `Object.assign` not protect these references from further mutations. On the other other hand, assigning a true `Fakemon` instance via `Object.assign` to another (even empty) object makes the latter not a `Fakemon` instance. It will remain a shallow copy, an object which holds references to deeper nested properties of its `Fakemon` instance source object. – Peter Seliger May 16 '22 at 15:58
  • 4/4 ... And making use of [`structuredClone`](https://developer.mozilla.org/en-US/docs/Web/API/structuredClone) does not solve the problem either for one would loose the anyway dispensable `pushFakemon` method. Thus the question arises ... What is the constructor good for. There are neither prototypal methods nor subclassing involved. My final suggestion is ... _"Use a fakemon factory"_. – Peter Seliger May 16 '22 at 16:05
  • Using structuredClone worked, thanks :) – Kristian Gjertsen May 17 '22 at 22:09
  • _"Using structuredClone worked"_ ... There must have been some other changes as well. Invoking `structuredClone` will fail (throw an exception) with instances of the OP's original `Fakemone` constructor function due to the method implementation of `pushFakemon`. – Peter Seliger May 17 '22 at 23:27
  • I did change `this.pushFakemon = function() { fakemonSamling.push(this); } this.pushFakemon();`, to`fakemonSamling.push(this);`. I only changed it because i did not need it, turns out i would not work otherwise, as you said. – Kristian Gjertsen May 19 '22 at 00:05

1 Answers1

0

Apparently the OP has/had problems with two of the design choices.

player1.push(Object.assign(temp1, randomFakemon())); ... picks a random true Fakemon instance from the OP's fakemon collection.

Object.assign treats the source like an ordinary object. It assigns just the source object's enumerable own properties to the target object. Thus, nested, non primitive properties still will hold references to their's sources. The target object also will not change its type. It will not all of a sudden be an instance of Fakemon.

The just described problem can be solved with a reliable own implementation of a clone function or, if available, by a structured clone.

With the cloning approach a constructor, though it create own types, but does neither extend/subclass nor implement prototypal methods, renders useless.

The only (own) method from the OP's example code is dispensable anyway, thus the suggested solution is a code refactoring towards a fakemon factory function and the usage of clone functionality.

function createFakemon(base, attack1, attack2) {
  const fakemon = Object.assign({
      health: 1000,
    }, base, {
      en: { ...attack1 },
      to: { ...attack2 },
    });
  fakemonCollection.push(fakemon);

  return fakemon;
};
const fakemonCollection = []; // Dansk: fakemon samling.

function getRandomFakemon() {
  return fakemonCollection[
    Math.floor(Math.random() * fakemonCollection.length)
  ];
}

const cloneDataStructure = (
  ('function' === typeof structuredClone) && structuredClone ||
  (value => JSON.parse(JSON.stringify(value)))
);

const fakemon1 = createFakemon({
  navn: 'BatCat',
  type: 'Flying',
  attackPower: [10, 50],
  sources: ['batFront.png', 'batBack.png'],
}, {
  navn: 'Blood Suck',
  force: [25, 38, 60],
  antall: 10,
}, {
  navn: 'Wing Slap',
  force: [10, 17, 25],
  antall: 20,
});
const fakemon2 = createFakemon({
  navn: 'Muffin Time',
  type: 'Normal',
  attackPower: [15, 45],
  sources: ['cupcakeFront.png', 'cupcakeBack.png'],
}, {
  navn: 'Frosting cover',
  force: [10, 17, 25],
  antall: 20,
}, {
  navn: 'Cake stomp',
  force: [40, 50, 60],
  antall: 5,
});

// create a(ny) player's fakemon as true clone
// of a randomly picked `fakemonCollection` item.
const playerFakemon = cloneDataStructure(
  getRandomFakemon()
);
// change a single attack value.
playerFakemon.en.navn = 'Foo Bar';
/*
  Thus instead of ...

  player1.push(Object.assign(temp1, randomFakemon()));

  ... one now would do ...

  player1.push(cloneDataStructure(getRandomFakemon()));
*/
console.log({
  // no mutation at any of the collection's fakemon items.  
  fakemonCollection,
  // single different value for `playerFakemon.en.navn`.
  playerFakemon,
});
.as-console-wrapper { min-height: 100%!important; top: 0; }
Peter Seliger
  • 11,747
  • 3
  • 28
  • 37
  • This a better approach than mine, but changing Object.assign to structuredClone did the job. – Kristian Gjertsen May 17 '22 at 22:11
  • @KristianGjertsen ... _"changing Object.assign to structuredClone did the job"_ ... I doubt that. There definitely were some other changes too. Invoking `structuredClone` will fail (throw an exception) with instances of the OP's original `Fakemone` constructor function due to the method implementation of `pushFakemon`. – Peter Seliger May 17 '22 at 23:29