youvsvirus / youvsvirus-unity

Unity version of the you vs virus game.
GNU General Public License v3.0
1 stars 1 forks source link

Discussing coroutines #125

Closed maccxs closed 4 years ago

maccxs commented 4 years ago

Hey Johannes, I have not yet fully grasped the coroutines and I am not sure I understand the problem. I just played around with the script, which works like that. I think what maybe happens: Game starts: player is in house, no coroutines Start: Player goes out: Trigger, starts coroutine InHouse a) player moves away, StopAllCoroutines called b) player goes in, coroutine OutHouse is called, InHouse still running --> only option: player goes out again which stop both coroutines since they are complete! --> and we are back at start -- In line 218 I now wrote yield return... which should make the Inhouse coroutine is terminated as soon as Outhouse is terminated

holke commented 4 years ago

Hi Margrit.

Yes, these coroutines are making my head spin and actually, i thought yesterday evening: Can't we get rid of them? But more on this later.

I would agree that the problem is either that suddenly all coroutines stop, or that the inhouse and outhouse routine are running at the same time. What i read about coroutines, i understand it the following way: Unity builds a stack of the running coroutines and if i call StopCoroutine(FunctionA), then the last instantiated Coroutine of this Function is stopped. Any others may still be running. I would also guess, that if we do a:

StartCoroutine(FunA())

FunA() {
   StartCoroutine(FunB());
   SomethingElse;
}

FunB() {
  StopCoroutine(FunA());
  SomethingElseB;
}

Then FunB will stop itself and SoemthingElseB will never get executed.

holke commented 4 years ago

Ok, so i had an idea yesterday evening and when i got up this morning i started working on it and think i have a concept that would work and get rid of the coroutines, resolving our mess. It was only while i was working on this, that i saw you discussion here. Also i think this manages the levelsettings better and should clear all the "if levelname == 'levelgethome'" statements.

What i propose is:

House has vars:

House provides functions:

Endlevelcontroller provides:

If we have these, the following Update routine should take care of everything. No need for subroutines.

void Update ()
{
  // Only do this every 0.1 secs or so to increase performance
  if (close_to_home) {
    // Only do smth if the player is close
    if (in_house) {
    // We are inside, check space to get outside
    if (SpaceIsPressed) {
              set_player_outside();
    }
    }
    else if (EndLevelController.isPlayerAllowedHome()) {
        // We are not inside, but are allowed to enter
        if (!enter_with_space || SpaceIsPressed) {
       // Either we are in enter-with-space mode and the space key was pressed,
       // or we are in just-enter mode.
       // we enter the house
       set_player_inside();
       EndLevelController.NotifyPlayerHome();
    }
    }
  }
}
maccxs commented 4 years ago

Hi @holke,

I am not set on the coroutines, so doing it another way is fine. I think my solution works too since either all coroutines are stopped or they run out themselves as I explained above. Or do we have two instance of the same coroutine? This is not what I observed at least.

What I am set on is however this:

/// <summary>
    /// Puts the player in the house
    /// Healthy player or exposed player depending on condition
    /// Notifies end level controller
    /// All with a little bit of time delay to make it look good
    /// </summary>
    private IEnumerator SetPlayerInHouse(GameObject p)
    {
        // takes a little while for the player to get inside and disappear
        yield return new WaitForSeconds(0.3f);
        p.SetActive(false);
        // takes some more time until the player looks outside the window
        yield return new WaitForSeconds(1f);
        playerRend.sprite = endlevel.playerExposed ? Resources.Load<Sprite>("SmileyPictures/player_exposed") : Resources.Load<Sprite>("SmileyPictures/player_healthy");
        ShowPlayer();
        // a little bit later we notify the end level controller that the player is home
        yield return new WaitForSeconds(0.5f);
        endlevel.NotifyPlayerAtHome();
    }

I actuall introduced the coroutines to have the time delay between player going inside and appearing in the window. Doing it instantly looks stupid ;-)

maccxs commented 4 years ago

Btw, the inhouse and outhouse coroutines are allowed to run at the same time.

holke commented 4 years ago

I actuall introduced the coroutines to have the time delay between player going inside and appearing in the window. Doing it instantly looks stupid ;-)

Ah, i somehow forgot about this in my thoughts. You are right. We need the coroutines for the time delay.

To be honest, i did not have the chance to look at your code but will do so as soon as i can.

Btw, the inhouse and outhouse coroutines are allowed to run at the same time.

Why?

maccxs commented 4 years ago

I actuall introduced the coroutines to have the time delay between player going inside and appearing in the window. Doing it instantly looks stupid ;-)

Ah, i somehow forgot about this in my thoughts. You are right. We need the coroutines for the time delay.

To be honest, i did not have the chance to look at your code but will do so as soon as i can.

Btw, the inhouse and outhouse coroutines are allowed to run at the same time.

Why?

I think: If Cor A calls Cor B, Cor A will continue to run until Cor B is finished. As long as both are terminated at some point, I don't think it's a problem they they run concurrently for a fixed time, not indefinitely.

holke commented 4 years ago

i still dont understand why they should run at the same time in our example. Either you want to enter the house or exit it. But not both at the same time?

maccxs commented 4 years ago

They don't have to but it just works like this. Maybe should make a telephone call.

maccxs commented 4 years ago

We could also just have a mixture of your ideas and the coroutine only for the time delay

holke commented 4 years ago

Sure. I will check your new code first and if it works, then we dont need to do anything else.

holke commented 4 years ago

I tried your code now. I can still be inside and outside of the house at the same time, though :/

maccxs commented 4 years ago

The Player is inside and Outside at the Same Time?

Von meinem Telefon gesendet

Am 29.05.2020 um 18:13 schrieb holke notifications@github.com:

 I tried your code now. I can still be inside and outside of the house at the same time, though :/

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

holke commented 4 years ago

I am working on a hybrid version of your and mine ideas now

holke commented 4 years ago

The Player is inside and Outside at the Same Time?

Von meinem Telefon gesendet

Am 29.05.2020 um 18:13 schrieb holke notifications@github.com:

 I tried your code now. I can still be inside and outside of the house at the same time, though :/

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

Yes he is.

maccxs commented 4 years ago

Yes he is.

I think I do not really understand the problem, but I believe there is one :-) Ich glaube ich habe ein Brett vorm Kopf. It's like if you say: The player is out and in at the same time: We have two instances of the player in and out of the house? This cannot really be the problem. Maybe a phone call could solve this faster than writing messages back and forth :-) Nevertheless, a combination of your idea and using the coroutine just for time delay should hopefully work. Good luck!

holke commented 4 years ago

I now implemented the hybrid version. The update routine is now the only one that is allowed to spawn coroutines and only does so, if no other coroutine runs.

I also fixed problems that i had with the triggers, which may be linked to bugs we had in the past here. The OnTriggerEnter/Exit functions were called by all the triggers associated with PlayerHouse, which means also the Door trigger for example.

I now installed a new trigger that is a suboject of the house with its own script. The purpose of this trigger is to tell the house when the player is close and may enter. The code in the script is more or less the same as it was for the house, but since it is now part of the new subobject instead, there is no danger of it being called by any other triggers of the house (such as the door trigger). Note, the final form (circle or poly or whatever) of this trigger may stil be changed. It is currently a circle since i first wanted to see if the concept works at all.

Please have a look at this and whether it now works and you like it.

I did not clean up the levels in this commit, since i want to finish the house first. So, for example you will now see the "press space to exit" canvas in the mask level all the time. I am aware of this and will properly adjust this soon.

I am off for today and dont know whether i can do something tomorrow. If you want to speak in person about this, we should do it on monday.

Best wishes :)

holke commented 4 years ago

Yes he is.

I think I do not really understand the problem, but I believe there is one :-) Ich glaube ich habe ein Brett vorm Kopf. It's like if you say: The player is out and in at the same time: We have two instances of the player in and out of the house? This cannot really be the problem. Maybe a phone call could solve this faster than writing messages back and forth :-) Nevertheless, a combination of your idea and using the coroutine just for time delay should hopefully work. Good luck!

We did not have 2 instances of the player. But the normal player instance was active and could move, while the little sprite of the player was looking outside of the window of the house.

maccxs commented 4 years ago

Hi, this seems to work really good now (and I can say so because I now understand the problem) :-) because I now saw the issue... Concerning the form of the trigger: I wanted the door to be the trigger that is used to enter the house. Maybe something got mixed up there but I just thought it to easy to enter throught the roof or anything like that! I wish you a nice and happy weekend. And Monday is for relaxing as well. So only do youvsvirs if unity saves your work... mmh maybe we should get this https://assetstore.unity.com/packages/tools/utilities/autosave-43605 ?

holke commented 4 years ago

Happy to hear that it works for you as well :)

As I said we can still change the form of the trigger, let’s discuss this next week. Have a nice weekend :-)

holke commented 4 years ago

Okay. I reworked it a bit again since it did not work already for the other levels. What i changed:

holke commented 4 years ago

Somehow the CreateHumans scripts now need to actively set the Player object to active when the player does not start inside the house. I dont really understand why. But thats how it is now.

holke commented 4 years ago

I will merge this now into the mask level to finish the mask level. But you can still review here and i will update the mask level with the changes.