youvsvirus / youvsvirus-unity

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

Leveldemo2 #127

Closed maccxs closed 4 years ago

maccxs commented 4 years ago

Leveldemo integrated in new master.

holke commented 4 years ago

I think the white text on the canvas is hard to read due to the white banners of the demo. In other levels i also sometimes find the white text hard to read. Maybe we can increase the contrast, or decrease the transparency of the canvas background or change the text color? We should do this consistently in all levels.

holke commented 4 years ago

If i get infected and then retry the game is paused

EDIT: This happened on the other branch, sorry

holke commented 4 years ago

I think the number of infected NPCs should be higher. I have the feeling i can run into some of these guys for a long time until i am infected.

maccxs commented 4 years ago

Would it be possible to make all further comments in exitkey branch? Everything in here is also in there? Or do you prefer this one?

maccxs commented 4 years ago

Actually, I would also like to integrate your suggestions in the exitkey branch. Would that be okay?

maccxs commented 4 years ago

So abandon this pull request? Or do you a have a valid argument against it?

maccxs commented 4 years ago

But don't close it.

maccxs commented 4 years ago

I think the number of infected NPCs should be higher. I have the feeling i can run into some of these guys for a long time until i am infected.

I made 2 more infectious, also you have now a 40% change of getting infected by propaganda from sprites without sign, integrated in exitkey-branch.

maccxs commented 4 years ago

I think the white text on the canvas is hard to read due to the white banners of the demo. In other levels i also sometimes find the white text hard to read. Maybe we can increase the contrast, or decrease the transparency of the canvas background or change the text color? We should do this consistently in all levels.

See pull request #131

maccxs commented 4 years ago

When the npcnumber was public in createpopulationleveldemo, I could no longer adapt the poupulation number within hte script, i.e. if I said public NPCNumber = 500, it was still 42 in the editor. That's why I changed it back. Also, this is maybe not something that should be allowed to change in the editor because it is inherent to each individual scene and the difficulty associated with it?

holke commented 4 years ago

When the npcnumber was public in createpopulationleveldemo, I could no longer adapt the poupulation number within hte script, i.e. if I said public NPCNumber = 500, it was still 42 in the editor. That's why I changed it back. Also, this is maybe not something that should be allowed to change in the editor because it is inherent to each individual scene and the difficulty associated with it?

My reasoning was actually because it is different in every level it should be public. Because this way we can easily adapt the number of NPCs for the level in the scene editor when we plug the level together, without duplicating the script or other hassles. Say we do 2 levels that follow the same createpopulation logic (since some levels are essentially the same, this is very probable to happen, like for example mask and gethome level), but in one level we want 10 and in the other 20 NPCs. If this number is not public then we have to create a copy of the script for each level - with the only difference of the number of NPCs - instead of being able to use the same script. Which also means that if we need to tweak smth for the create humans routine in one level, we need to be aware that we also have to tweak it in the other level.

EDIT: Or we have to do some "If levename == STRING" stuff in the script again to set the correct NPC number.

holke commented 4 years ago

Would it be possible to make all further comments in exitkey branch? Everything in here is also in there? Or do you prefer this one? Actually, I would also like to integrate your suggestions in the exitkey branch. Would that be okay? So abandon this pull request? Or do you a have a valid argument against it?

Yes it would be possible. But i am not a fan of it. I´d rather be able to discuss each feature in its own discussion an not mix them together. This makes reviewing and editing much cleaner. For me this is the first time i am in the situation of needing to review features that build on top of non-reviewed features. There is probably a good git-way to do this without mixing up the comments and we should find and adopt it for the future (after release).

maccxs commented 4 years ago

When the npcnumber was public in createpopulationleveldemo, I could no longer adapt the poupulation number within hte script, i.e. if I said public NPCNumber = 500, it was still 42 in the editor. That's why I changed it back. Also, this is maybe not something that should be allowed to change in the editor because it is inherent to each individual scene and the difficulty associated with it?

My reasoning was actually because it is different in every level it should be public. Because this way we can easily adapt the number of NPCs for the level in the scene editor when we plug the level together, without duplicating the script or other hassles. Say we do 2 levels that follow the same createpopulation logic (since some levels are essentially the same, this is very probable to happen, like for example mask and gethome level), but in one level we want 10 and in the other 20 NPCs. If this number is not public then we have to create a copy of the script for each level - with the only difference of the number of NPCs - instead of being able to use the same script. Which also means that if we need to tweak smth for the create humans routine in one level, we need to be aware that we also have to tweak it in the other level.

EDIT: Or we have to do some "If levename == STRING" stuff in the script again to set the correct NPC number.

I agree, we need to to this properly. But maybe this can also be done after the relase for each level consitently`?

maccxs commented 4 years ago

Would it be possible to make all further comments in exitkey branch? Everything in here is also in there? Or do you prefer this one? Actually, I would also like to integrate your suggestions in the exitkey branch. Would that be okay? So abandon this pull request? Or do you a have a valid argument against it?

Yes it would be possible. But i am not a fan of it. I´d rather be able to discuss each feature in its own discussion an not mix them together. This makes reviewing and editing much cleaner. For me this is the first time i am in the situation of needing to review features that build on top of non-reviewed features. There is probably a good git-way to do this without mixing up the comments and we should find and adopt it for the future (after release).

Maximum entropy ;-)

maccxs commented 4 years ago

Would it be possible to make all further comments in exitkey branch? Everything in here is also in there? Or do you prefer this one? Actually, I would also like to integrate your suggestions in the exitkey branch. Would that be okay? So abandon this pull request? Or do you a have a valid argument against it?

Yes it would be possible. But i am not a fan of it. I´d rather be able to discuss each feature in its own discussion an not mix them together. This makes reviewing and editing much cleaner. For me this is the first time i am in the situation of needing to review features that build on top of non-reviewed features. There is probably a good git-way to do this without mixing up the comments and we should find and adopt it for the future (after release).

Maximum entropy ;-)

And sorry again for the chaos.

holke commented 4 years ago

I agree, we need to to this properly. But maybe this can also be done after the relase for each level consitently`?

Yes, that is what i wanted to imply. Do it now on the qkey branch to quickly finish the next release and restructure our workflow afterwards.