codecombat / codecombat

Game for learning how to code.
http://codecombat.com
MIT License
7.95k stars 4.12k forks source link

magic.CastsRaiseDead buggy #2038

Closed mkronenfeld closed 9 years ago

mkronenfeld commented 9 years ago

I tried several ways to cast this spell, but am not able to cast the "raise-dead" spell from the Unholy Tome item.


// Cast spell like in the function documentation
if (this.canCast("raise-dead")) {
    this.cast("raise-dead");
}

// Fix Your Code
// You need something to cast upon

// Select an enemy or corpse in range (< 20m) as target
if (this.canCast("raise-dead")) {
    this.cast("raise-dead", target);
}

// Fix Your Code
// Unhandled error: TypeError: Cannot read property 'maxHealth' of undefined;

nwinter commented 9 years ago

Just trying a little fix here, although I don't have a ton of time to test right now (maybe more later); can you clear your browser cache and let me know what behavior you see when not using an argument now?

mkronenfeld commented 9 years ago

Unfortunately the problem still occurs in all of my browsers.

Fix Your Code Line 12: You need something to cast upon.

// The Forest - Ogre Encampment.
// Miss Hushbaum + Unholy Tome III
var enemy;

loop {
    enemy = this.findNearestEnemy();

    if (enemy) {
        this.cast("drain-life", enemy);
    } else {
        if (this.canCast("raise-dead")) {
            this.cast("raise-dead");
        }
    }
}
nwinter commented 9 years ago

@nemoyatpeace heads up–not that I expect you to fix it, but just to let you know that this is also something that needs help to make Raise Dead work right. (How were you testing it the other day?)

nemoyatpeace commented 9 years ago

Yes, I had this issue, but then I think I just cast on myself and it would cast. The problem was that they corpse would stand up, then drop again.

I'm wondering if maybe the raise dead shouldn't have a duration, since it only raises one corpse anyway, I think it should be raised at half health until killed again. This might also fix the issue of him just dying again.

nemoyatpeace commented 9 years ago

OK, so now RaiseDead has real problems, this might be because I'm attempting to cast when there are no enemies around.

image

if self.canCast("raise-dead", self):
        self.cast("raise-dead",self)
        self.say("Arise!")

Nowhere in my code do I look at maxHealth, so the error must be raised by raise dead.

nemoyatpeace commented 9 years ago

OK, I submitted a patch to fix that last error. Corpses do seem to attack ogres now, but they are attacked by humans and thus they die before they are able to do anything. (I'm testing on siege of stonehold). I guess something needs to be adjusted in the targeting AI so that it actually looks at the current team rather than just the original team.

nemoyatpeace commented 9 years ago

Looking at this:

  getEnemies: ->
    # Optimize
    return [] unless @canSee
    enemies = []
    for thang in @allianceSystem.allAlliedThangs
      if thang.superteam isnt @superteam and thang[@significantProperty] and @canSee thang
        enemies.push thang
    enemies

Looks like I need to change the superteam of the corpses, is that possible?

Yep, it is! I changed the superteam and now the raised corpses attack my enemies and are attacked by my friends.

I still think that maybe we should remove the duration, though I think they were all killed before the time would have run out.

nwinter commented 9 years ago

Let's test it in a few more levels before removing the duration; I don't want to accidentally make something that's too overpowered and scales up with the difficulty of the enemies, then have to nerf it.

nemoyatpeace commented 9 years ago

Hmmm, so I just tried it out on Odd Sandstorm, killed two munchkins, then cast raise-dead. The raised munchkin got killed by a yak causing the quest to fail because a human died.

I wonder if things will work properly if the superteam is changed, but the team left alone. Nope, if I leave team alone, the raised ogres just die again instantly, I tested again on Seige of Stormhold.

Not sure if it is really an issue that the ogre counts as a human that can't be killed. But could the mission be a specific count of humans that survive instead of all humans so that the ogre raised to human could die without failing the quest.

nemoyatpeace commented 9 years ago

Hmmm, this caused issues in Rich Forager as well. I raised two different ogres, after the proper period of time they both died again, then the quest ended with this failure: "Your hero must survive. (1/2)"

While they were dead, they did flip back and forth some like earlier corpses did though.

image

nwinter commented 9 years ago

Could we try reverting their team/superteam one frame (@world.dt seconds) before they die again, and maybe replace their die method with one that switches the team back before calling the original die method?

nemoyatpeace commented 9 years ago

Going to the original issue here, this seems to be where the error is happening, not sure what is wrong with it though.

From magic.Casts:

 cast: (spell, target, methodName) ->
    target ?= @ if spell.name in ['time-warp', 'raise-dead', 'windstorm']  # TODO: figure out how to not have to hard-code the self target list check in here.
    unless target
      throw new ArgumentError "You need something to cast upon.", methodName or 'cast', "target", "object", target
nemoyatpeace commented 9 years ago

Is there a removeEffect method? Can I create a raisedDie function like this:

  raisedDie: ->
    #What happens when the unit dies again.
    @.removeEffect 'confuse'
    @.die() 

Connected to this, does the name of the effect matter? This is from the raise-dead function:

if corpse 
        effects = [
          {name: 'confuse', duration: @raiseDeadDuration, reverts: true, setTo: false, targetProperty: 'dead'}
          {name: 'confuse', duration: @raiseDeadDuration, reverts: true, setTo: @raisedChooseAction, targetProperty: 'chooseAction'}
          {name: 'confuse', duration: @raiseDeadDuration, reverts: true, setTo: corpse.maxHealth / 2, targetProperty: 'health'}
          {name: 'confuse', duration: @raiseDeadDuration, reverts: true, factor: 0.5, targetProperty: 'maxSpeed'}
          {name: 'confuse', duration: @raiseDeadDuration, reverts: true, setTo: 'humans', targetProperty: 'team'}
          {name: 'confuse', duration: @raiseDeadDuration, reverts: true, setTo: 'humans', targetProperty: 'superteam'}
          {name: 'confuse', duration: @raiseDeadDuration, reverts: true, setTo: 'corpse', targetProperty: 'type'}
        ]
        corpse.addEffect effect, @ for effect in effects

Should the name: 'confuse' be changed to something like name: 'raised'?

nwinter commented 9 years ago

confuse here does two things:

  1. It makes the confuse Mark appear above the Thang's head, and so must be confuse.
  2. It sort of identifies the "raise-dead" effect (except because we want to use the confuse Mark, we change the unique string. (We don't have a raised Mark.)

If we want to remove effects, we can do it like this:

corpse.effects = (e for e in corpse.effects when e.name isnt 'confuse')

Downside: if the corpse was confused from another effect that causes confusion, it'll lose that effect, too. I don't think that's something to be concerned about here, though.

Also, @.die() is better written as @die()–CoffeeScript @ can mean either this or this..

nemoyatpeace commented 9 years ago

So, I added the line about effects:


  raisedDie: ->
    #What happens when the unit dies again.
    @effects = (e for e in @effects when e.name isnt 'confuse')
    @die()

and now I get this error basically right after I raise the dude:

image

nemoyatpeace commented 9 years ago

Should I be able to raise an opponent hero? I'm trying in Treasure Grove, but it doesn't work, I cast, but he doesn't get raised.

nwinter commented 9 years ago

The infinite recursion would be because die is now raisedDie. You want to have a reference to originalDie and then call that within raisedDie.

Probably try against something that's not a hero; the heroes are much more complicated in how they chooseAction.

nemoyatpeace commented 9 years ago

I'm not sure where to go to finish fixing, and will have to focus my time on lesson plans now, hopefully someone can finalize for us.

nwinter commented 9 years ago

Just to catch me up: what currently is still not working about it?

nemoyatpeace commented 9 years ago

Well, it seems touchy, sometimes it casts and other times it doesn't. I felt like I had it working on one level, but not another. Hard to test on different levels though as I always have to open it in the editor to equip the item.

nwinter commented 9 years ago

Think I got it all fixed up.

nemoyatpeace commented 9 years ago

So, I just found a bug with this. If a corpse is targeted by an enemy, that enemy seems to continue to target the corpse even after it has died again.

image

Kinda fun watching Ogres pound on dead ogres, but probably not in the plan!

arghyapolley commented 9 years ago

write down your code

nemoyatpeace commented 9 years ago

That is not a helpful comment, this thread is discussing issues with raise dead. My code doesn't play into it. Why is it that your only comments on here are two inappropriate instructions to write down full code?

nwinter commented 9 years ago

Is this when the corpse dies again because it is killed, or because it times out?

nemoyatpeace commented 9 years ago

In this case, he was killed, not sure about if it times out. Don't know how to test, they would need to time out while being attacked. The corpse is not attacked by new enemies, only by the enemies that were already attacking when it died.

nemoyatpeace commented 9 years ago

I added findCorpses, it seems to work.

On another note, if you raise a thrower they die insanely fast, basically a waste. Would be cool if raise dead always raised the most powerful or if it would raise a certain power, so if throwers, it will raise 2 or 3 of them so they at least survive enough to throw a spear before they just drop again.

nwinter commented 9 years ago

Oh yeah, raising a certain power of corpses instead of a certain number would make so much more sense! We should change it to do that instead. It could raise until it had exceeded the power, since if we let it go over, we won't have disappointing raises where it, say, gets one thrower and then can't get the brawler and doesn't do anything afterwards.

Problem: power table is in the Referee Component. I guess we should move it somewhere else so that CastsRaiseDead can access it. But where?

nemoyatpeace commented 9 years ago

misc.PowerTable?

nwinter commented 9 years ago

Ah, I just realized that the power table should go into the Existence or Combat Systems, since you can always access properties of Systems from Components.

nwinter commented 9 years ago

I did see the bug with enemies failing to keep fighting after targeting a corpse, so that's still happening.

nwinter commented 9 years ago

I've fixed the bugs with dead units not returning to death properly if they time out instead of being killed again, and with living units continuing to target them. I've also moved the buildTypePowers to the Existence System. Want to take over and change it to raise a certain power of dead units?

nemoyatpeace commented 9 years ago

Sure, but will have to wait til Spring Break most likely.