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

Fix OnAllRemovedFromWorld only triggering once #15680

Merged
merged 1 commit into from Jan 22, 2019

Conversation

abcdefg30
Copy link
Member

We trigger all the other triggers like OnRemovedFromWorld multiple times, except the "on killed" ones (obviously) and the "on captured" triggers (I could fix those as well, we usually don't need this however).

@abcdefg30
Copy link
Member Author

Rebased.

@pchote pchote added this to the Next Release milestone Nov 25, 2018
Copy link
Member

@pchote pchote left a comment

Choose a reason for hiding this comment

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

Code changes LGTM, just one minor nit.

Can you please provide a testcase that demonstrates the bug and fix?

OpenRA.Mods.Common/Scripting/Global/TriggerGlobal.cs Outdated Show resolved Hide resolved
@pchote
Copy link
Member

pchote commented Dec 9, 2018

Ping @abcdefg30

@pchote
Copy link
Member

pchote commented Dec 20, 2018

Do you have a testcase that demonstrates the bug and fix?

@pchote
Copy link
Member

pchote commented Dec 31, 2018

Deferring to Next + 1 so this won't hold up the playtest. We can pull this back if someone can provide a testcase so we can merge this in time.

@pchote pchote modified the milestones: Next Release, Next + 1 Dec 31, 2018
@abcdefg30
Copy link
Member Author

Testcase: Replace SetupAlliedUnits in Allies03b with the following

SetupAlliedUnits = function()
	Actor.Create("e7", true, { Owner = player, Location = TanyaWaypoint.Location + CVec.New(0, 2), Facing = 128 })
	Tanya = Actor.Create(TanyaType, true, { Owner = player, Location = TanyaWaypoint.Location, Facing = 128 })
	Rifle1 = Actor.Create("e1", true, { Owner = player, Location = TanyaWaypoint.Location + CVec.New(0, 1) })
	Rifle2 = Actor.Create("e1", true, { Owner = player, Location = TanyaWaypoint.Location + CVec.New(1, 0) })

	if TanyaType == "e7.noautotarget" then
		Trigger.AfterDelay(DateTime.Seconds(2), function()
			Media.DisplayMessage("According to the rules of engagement I need your explicit orders to fire, Commander!", "Tanya")
		end)
	end

	Camera.Position = Tanya.CenterPosition

	Trigger.OnAllRemovedFromWorld({ Tanya, Rifle1, Rifle2 }, function()
		Media.DisplayMessage("Retreat is not an option!")
	end)

	InsertionHeli.Wait(DateTime.Seconds(3))
	InsertionHeli.UnloadPassengers()

	Tanya.Wait(DateTime.Seconds(1))
	Tanya.EnterTransport(InsertionHeli)

	Trigger.AfterDelay(DateTime.Seconds(5), function()
		Rifle1.EnterTransport(InsertionHeli)
		Rifle2.EnterTransport(InsertionHeli)

		Trigger.AfterDelay(DateTime.Seconds(5), function()
			Tanya.EnterTransport(InsertionHeli)
		end)
	end)

	--InsertionHeli.Move(InsertionHeliExit.Location)
	--InsertionHeli.Destroy()

	Trigger.OnKilled(Tanya, function() player.MarkFailedObjective(TanyaSurvive) end)
end

You won't have do anything other than watching for the message "Retreat is not an option!". It should only appear once one Tanya and both rifles are inside the transport. (On bleed you will see the message with only the two rifles inside the transport but both Tanyas outside.)

@pchote
Copy link
Member

pchote commented Jan 22, 2019

Thanks, LGTM.

@pchote pchote merged commit 3e93242 into OpenRA:bleed Jan 22, 2019
@abcdefg30 abcdefg30 deleted the onAllRemoved branch January 23, 2019 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants