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
Ported Allies 01 to the New Lua API #6377
Conversation
@@ -259,11 +258,6 @@ Actors: | |||
Owner: Greece | |||
Health: 1 | |||
Facing: 128 | |||
Harvester: harv |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kind of prefer the current setup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I don't. Instead of inventing Harvester.Harvest in Lua and micromanaging instructions for the unit, I can just let automation kick in like we do for the RA desert shellmap and all TD mission. That is far more elegant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I explicitly wanted the harvester to start where it did in the original :/
On 31/08/2014 11:42 p.m., Matthias Mailänder wrote:
In mods/ra/maps/allies-01/map.yaml:
@@ -259,11 +258,6 @@ Actors:
Owner: Greece
Health: 1
Facing: 128
- Harvester: harv
Well, I don't. Instead of inventing Harvester.Harvest in Lua and
micromanaging instruction, I can just let automation kick in like we
do for the RA desert shellmap and all TD mission.—
Reply to this email directly or view it on GitHub
https://github.com/OpenRA/OpenRA/pull/6377/files#r16934316.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can try to teleport it to Location: 72,60 but why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not? The harvester harvesting near your units at the start of the
mission was pretty notable in the original.
On 31/08/2014 11:52 p.m., Matthias Mailänder wrote:
In mods/ra/maps/allies-01/map.yaml:
@@ -259,11 +258,6 @@ Actors:
Owner: Greece
Health: 1
Facing: 128
- Harvester: harv
I can try to teleport it to Location: 72,60 but why?
—
Reply to this email directly or view it on GitHub
https://github.com/OpenRA/OpenRA/pull/6377/files#r16934350.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't really find a quick and clean way to do it. You can still see the harvester working during the mission. I believe the harvester placement was a workaround for limitations in the Westwood engine which did not free actors on pre-placed buildings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also note:
Harvester: harv
Location: 72,60
[...]
Actor33: proc
Location: 73,58
in map.yaml.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ended up porting the FindResources activity from legacy to the new Lua API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a significant difference when you consider the map size and that
with FreeActor the harvester will have to drive all the way around the
refinery to access the ore.
On 1/09/2014 2:26 a.m., Matthias Mailänder wrote:
In mods/ra/maps/allies-01/map.yaml:
@@ -259,11 +258,6 @@ Actors:
Owner: Greece
Health: 1
Facing: 128
- Harvester: harv
Also note:
Harvester: harv Location: 72,60
[...]
Actor33: proc
Location: 73,58in map.yaml.
—
Reply to this email directly or view it on GitHub
https://github.com/OpenRA/OpenRA/pull/6377/files#r16935172.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Did not get that you meant this was related to timing.
435179d
to
a588726
Compare
Beacon: beepslct | ||
Alert: buzzy1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alert is a bit too generic for this. There are other alert sounds like bleep6.aud.
82c320f
to
42047e5
Compare
The ore management "hack" was deliberate, to show the ore silos being filled up and depleted. |
Depleted by what? There is no production in the enemy base. |
It was to give the impression that the enemy as a whole was spending their cash, not necessarily at the base you were infiltrating. There's also more activity going on in the base. |
I see, but I think the mission is far too short to note such a detail. |
42047e5
to
666f990
Compare
This carries over to other missions as well. |
I agree with Scott here that we need the TickTakeOre hack sooner or later again. Might as well just add it now. A couple of issues I noted during gameplay:
|
end | ||
local i = 1 | ||
Utils.Do(CruisersReinforcements, function(cruiser) | ||
local ca = Actor.Create(cruiser, true, { Owner = england, Location = SouthReinforcementsPoint.Location }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that we can do addition with CPos and CVec values, maybe you could change this so the cruisers don't all start on top of one another
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. Done.
666f990
to
3c78a89
Compare
Adding resources just to remove them every tick sounds super braindead to me, but you are the reviewers I have to satisfy. I also guess that my protest has been noted that spawning a harvester right next to the refinery and manually telling the it to find resources is more than complicated so I also give up on that. Updated. |
It is, but it's needed to avoid the "harvester unloading forever" animation. If that worked differently, we wouldn't need it (for all I care, anyway). Anyway, this still doesn't quite work correctly when you lose. You can fix that by marking the Soviet objective as completed instead of marking Greece's objective as failed. (Background: the way you did it would have worked if you only had two combatants. But you have three, and the mission only ends when all non-combatants have a defined win state. When a player wins, he marks all his enemy's remaining objectives as failed. When you fail one of Greece's objectives instead, Greece has WinState.Lost and sets USSR's win state to Won, but England's win state is still undefined. When you do it the other way around, the USSR sets both Greece's as well as England's win state to Lost and the game ends. The other case - the player winning - works because, as I mentioned above, the winning player causes his enemy's remaining objectives to fail. This causes the enemy to lose. When a player loses, he sets his enemy's win state to Won. So Greece's win causes USSR to lose which causes England to win. Now all three players have a defined win state and the game ends. The only reason it's done this way is to ensure that all the checkboxes are crossed correctly when a player loses. And the reason why losing and winning is handled differently is because when a player wins, it means that his enemies failed their (remaining) objectives. But when a player loses, it does not automatically mean that his enemies have completed their objectives. It does mean that they have won, though.) |
The harvester unloading forever is also resolved by just making resources take no space. |
Yes, in this case, where the AI doesn't actually need the money. In other maps it will be different. |
You are in fact both right. In the second mission #6379 you will already need to harvest yourself so making ore generally take no space is a bogus workaround there. |
I did both now. |
3c78a89
to
f0716d9
Compare
|
||
[ScriptActorPropertyActivity] | ||
[Desc("Fly within the cell grid.")] | ||
public void Fly(CPos cell) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be called Move, to match what we have for ground units.
Disposed |
bf84d01
to
cbaecab
Compare
Maybe add what @obrakmann did on gdi01: Trigger.OnObjectiveCompleted(player, function(p, id)
Media.DisplayMessage(p.GetObjectiveDescription(id), "Objective completed")
end)
Trigger.OnObjectiveFailed(player, function(p, id)
Media.DisplayMessage(p.GetObjectiveDescription(id), "Objective failed")
end) |
this can be rebased now |
cbaecab
to
867ebce
Compare
Rebased and added the automatic textual objective notifications as suggested. |
👍 |
👍 |
Ported Allies 01 to the New Lua API
Definitely worth it. Going to finish #6379 now. |
Allies01: There is a prblem with the second objective, all civilians get killed right after the start of the mission :( |
There is a problem with your reflexes, you need to be quick to power off the tesla coils. =) |
lol okay ;) |
Changes include:
The ore management hack in TickIdle has been removed (as in the desert-shellmap).