Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed die event for heroes doesn't trigger on S27 #122

Merged
merged 3 commits into from Mar 31, 2023

Conversation

Toranks
Copy link
Contributor

@Toranks Toranks commented Mar 19, 2023

The solution was even simpler than what I told you on discord. Put the death events BEFORE the interrogation events, where the undead transformation event is included.
Also, moved die events to last breath on some character that must say a sentence when die, same as other characters. I think it was an omission or an oversight, but in cases like this, the died unit says his final sentence while being invisible.

Notes:

  • Draft because need testing. Debug mode has a secret trigger to make Grekulak come on any turn, must be useful.
    - {IS_LOYAL} is redundant, deleted just to reduce code

@Toranks Toranks marked this pull request as ready for review March 20, 2023 17:41
@cooljeanius
Copy link
Collaborator

The solution was even simpler than what I told you on discord. Put the death events BEFORE the interrogation events, where the undead transformation event is included. Also, moved die events to last breath on some character that must say a sentence when die, same as other characters.

So, the events that have been changed from die to last breath are used in more scenarios besides just S27, aren't they? Did you test any other scenarios with these changes besides Orannon?

I think it was an omission or an oversight, but in cases like this, the died unit says his final sentence while being invisible.

Notes:

* Draft because need testing. Debug mode has a secret trigger to make Grekulak come on any turn, must be useful.

* {IS_LOYAL} is redundant, deleted just to reduce code

isn't that also in #123?

@Toranks
Copy link
Contributor Author

Toranks commented Mar 26, 2023

In other scenarios you won't notice the difference because there are no dead unit transformations that I've seen.
Although of course we need to check all situations. Look for last_breath or die events related to these 3 characters on other scenarios.

isn't that also in #123?

I am going to undo it and move it there, I realized that I made this change in the branch that did not correspond

@cooljeanius
Copy link
Collaborator

In other scenarios you won't notice the difference because there are no dead unit transformations that I've seen. Although of course we need to check all situations. Look for last_breath or die events related to these 3 characters on other scenarios.

@nemaara can you think of any specific situations we might want to check?

@nemaara
Copy link
Owner

nemaara commented Mar 31, 2023

Not off the top of my head, do you want me to merge or check again?

@cooljeanius
Copy link
Collaborator

nvm I reviewed it myself and the changes to "last breath" events are correct; I even actually already refer to one of them as such even though it's currently a "die" event, lol

@cooljeanius cooljeanius merged commit 510c038 into nemaara:master Mar 31, 2023
@cooljeanius
Copy link
Collaborator

oh wait actually one other thing that I'm only realizing after merging: in the previous events when I split "last breath" events off from "die" ones, I made them 2 separate events: "last breath" for the message, and "die" for the "end level in defeat" part. We should probably do likewise here, as well.

cooljeanius added a commit to cooljeanius/A_New_Order that referenced this pull request Mar 31, 2023
change certain "die" events to "last breath" events; see nemaara/A_New_Order#122 by @Toranks
(untested)
@@ -1642,9 +1642,9 @@ Forces of Darkness will consume your mind."}

# A good portion of this scenario's code is actually in this FINAL_INTERROGATIONS macro;
# it is located in macros/ano-20macros.cfg:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh right, I meant to mention: if moving the macro, we should also move the comment associated with it, too

cooljeanius added a commit to cooljeanius/A_New_Order that referenced this pull request Mar 31, 2023
reorder macros; this is the other half of nemaara/A_New_Order#122 by @Toranks
(untested)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants