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

D2K - Add Harkonnen 7 #14426

Merged
merged 1 commit into from Dec 21, 2017

Conversation

Projects
None yet
7 participants
@MustaphaTR
Member

MustaphaTR commented Nov 26, 2017

OK, this one has something different than original. Lemme explain.

You capture the Atreides ConYard in this mission. In original game it would only give you Atreides High Tech Factory, other buildings were still Harkonnen. Here i changed it so you get all buildings in Atreides style. Because i think it makes more sense. But still build Harkonnen Combat Tank and Devastator and not Atreides Combat Tank and Sonic Tank to not break the way mission is played.

Making the above was a bit hacky as apperantly Player: actor can't provide prerequisites. I asked @pchote and he said we should rely on the hack for now.

[06:11:45] | <MustaphaTR>  pchote : it looks like Player actor can't provide Prerequisites (i  don't remember what the crash were, some time passed since i tested it  for Har7). Do you think a hacky solution like that is OK, or i should  try to fix the issue with Player: actor. https://github.com/MustaphaTR/OpenRA/commit/8fbabe43adc31e19fad54577078e06e9f14214c5
[06:12:31] | <MustaphaTR> I used that on Har7 to make player get Har Combat and Devastator while having Atr buildings.
[09:08:09] | <pchote> MustaphaTR: try and fix it properly
[09:11:25] | <MustaphaTR> ok
[09:18:24] | <pchote> MustaphaTR: actually, step 1 should really be: try and diagnose the real problen
[09:18:36] | <pchote> and then file a bug report if needed
[09:18:52] | <pchote> the type of fix or whether it should be fixed at all really depends on what the problem is
[09:20:21] | <MustaphaTR> i'll redo the crash and see what it says.
[09:26:57] | <MustaphaTR> pchote: this is the crash : https://gist.github.com/MustaphaTR/3b0651ebfe8d0eac8e02f0f9e213a12f
[09:28:33] | <pchote> ok, that makes sense
[09:28:40] | <pchote> that's going to be tricky to solve
[09:29:05] | <pchote> best to use your workaround for now

player_prerequisite actor is currently using camera artwork. I can
a) Complately get rid of editor only artwork for it.
b) Give it a custom artwork, but not sure what i should use.

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Nov 26, 2017

Member

I later realized that there was a much simpler workaround for the ProvidesPrerequisite problem. See #14428.

Member

pchote commented Nov 26, 2017

I later realized that there was a much simpler workaround for the ProvidesPrerequisite problem. See #14428.

@reaperrr reaperrr referenced this pull request Nov 26, 2017

Closed

Get the remaining Harkonnen missions into the playtest #14392

5 of 5 tasks complete
@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Nov 26, 2017

Member

Updated. Now depends on #14428.

Member

MustaphaTR commented Nov 26, 2017

Updated. Now depends on #14428.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Nov 26, 2017

Contributor

#14428 was merged, so technically needs rebase to be playable.

Contributor

reaperrr commented Nov 26, 2017

#14428 was merged, so technically needs rebase to be playable.

@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Nov 27, 2017

Member

Rebased.

Member

MustaphaTR commented Nov 27, 2017

Rebased.

@abcdefg30

This comment has been minimized.

Show comment
Hide comment
@abcdefg30

abcdefg30 Nov 28, 2017

Member

You capture the Atreides ConYard in this mission. In original game it would only give you Atreides High Tech Factory, other buildings were still Harkonnen. Here i changed it so you get all buildings in Atreides style. Because i think it makes more sense. But still build Harkonnen Combat Tank and Devastator and not Atreides Combat Tank and Sonic Tank to not break the way mission is played.

I like the decision to switch everything to Atreides artwork. Am not a fan of having Harkonnen units then though. Why would having the Sonic Tank break the way the mission is played?

Member

abcdefg30 commented Nov 28, 2017

You capture the Atreides ConYard in this mission. In original game it would only give you Atreides High Tech Factory, other buildings were still Harkonnen. Here i changed it so you get all buildings in Atreides style. Because i think it makes more sense. But still build Harkonnen Combat Tank and Devastator and not Atreides Combat Tank and Sonic Tank to not break the way mission is played.

I like the decision to switch everything to Atreides artwork. Am not a fan of having Harkonnen units then though. Why would having the Sonic Tank break the way the mission is played?

@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Nov 29, 2017

Member

It is not like this Devestator is a key factor in the mission, but 7th missions are meant to be when you get your IX units, for this case Devestator.

Member

MustaphaTR commented Nov 29, 2017

It is not like this Devestator is a key factor in the mission, but 7th missions are meant to be when you get your IX units, for this case Devestator.

@abc013

First of all, good work, the start wasn't easy for me :P.
It would be nice if the start units are facing down (towards the base you have to capture).

Show outdated Hide outdated mods/d2k/maps/harkonnen-07/rules.yaml Outdated
@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Nov 29, 2017

Member

It would be nice if the start units are facing down (towards the base you have to capture).

Original game had facings hardcoded. I see no problem with changing that for ORA.

Member

MustaphaTR commented Nov 29, 2017

It would be nice if the start units are facing down (towards the base you have to capture).

Original game had facings hardcoded. I see no problem with changing that for ORA.

@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Nov 30, 2017

Member

Updated, edited the briefing and changed initial unit's facing to south.

Member

MustaphaTR commented Nov 30, 2017

Updated, edited the briefing and changed initial unit's facing to south.

@ltem

Well done, especially the start was quite challenging, took me several tries.

I noticed that the Atreides (Main Base) won't use their Air Strike power and I think there was some clogging happening in the Atreides Small Base, which possibly assured my surviving.

While the clogging is probably out of scope for this PR, I would like to see Air Strike happening (if possible).

@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Dec 7, 2017

Member

I don't know if AirStrike is possible or not, if it is would be happy if anyone lets me know how.

Member

MustaphaTR commented Dec 7, 2017

I don't know if AirStrike is possible or not, if it is would be happy if anyone lets me know how.

@abcdefg30

This comment has been minimized.

Show comment
Hide comment
@abcdefg30

abcdefg30 Dec 7, 2017

Member

The AirstrikePower of an actor can be triggered using the
SendAirstrike(WPos target, bool randomize = True, int facing = 0) Lua method on the actor.

Member

abcdefg30 commented Dec 7, 2017

The AirstrikePower of an actor can be triggered using the
SendAirstrike(WPos target, bool randomize = True, int facing = 0) Lua method on the actor.

@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Dec 8, 2017

Member

Updated, AI now uses the Airstrike.

Member

MustaphaTR commented Dec 8, 2017

Updated, AI now uses the Airstrike.

@pchote pchote added this to the Next release milestone Dec 9, 2017

@ltem

This comment has been minimized.

Show comment
Hide comment
@ltem

ltem Dec 13, 2017

Contributor

Hey, @abc013 could you maybe re-review this PR?

Contributor

ltem commented Dec 13, 2017

Hey, @abc013 could you maybe re-review this PR?

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Dec 17, 2017

Contributor

I'm not a good player, but even at Easy difficulty this is still too hard for me. I don't mind this being more challenging than in the original, but I think I'd have less difficulty with the original's Normal... only got to build a wind trap and barracks before I got overrun by Corrino infantry from the south-east passage and atreides from the west at most 3 minutes into the mission.

I think at Easy, the AI should start production and attacks at least a minute later, possibly two.

Not sure whether we can manage that before the upcoming release, but I think for Next+1 at the latest, we should re-review difficulty of all missions and at least add the 25% cost/build speed reduction from the original to Easy mode, and possibly give players a little more time at the beginning of a mission, too.

Contributor

reaperrr commented Dec 17, 2017

I'm not a good player, but even at Easy difficulty this is still too hard for me. I don't mind this being more challenging than in the original, but I think I'd have less difficulty with the original's Normal... only got to build a wind trap and barracks before I got overrun by Corrino infantry from the south-east passage and atreides from the west at most 3 minutes into the mission.

I think at Easy, the AI should start production and attacks at least a minute later, possibly two.

Not sure whether we can manage that before the upcoming release, but I think for Next+1 at the latest, we should re-review difficulty of all missions and at least add the 25% cost/build speed reduction from the original to Easy mode, and possibly give players a little more time at the beginning of a mission, too.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Dec 17, 2017

Contributor

I noticed one thing that looks like a bug (killing/capturing high-tech factory cuts off all attacks, not just Corrino).
Apart from that, as mentioned above, 'Easy' is simply not easy enough for "rather-untalented-but-somewhat-experienced" players like me, and possibly people relatively new to RTS as well.
I don't care if Normal and Hard are designed to provide a challenge even to people with a lot more talent/practice, but 'Easy' simply shouldn't be unbeatable for a player like me, and currently it is.

Apart from those 2 issues the mission seemed fine (played it through with the help of some modded supertanks), so 👍 after these issues are adressed.

Contributor

reaperrr commented Dec 17, 2017

I noticed one thing that looks like a bug (killing/capturing high-tech factory cuts off all attacks, not just Corrino).
Apart from that, as mentioned above, 'Easy' is simply not easy enough for "rather-untalented-but-somewhat-experienced" players like me, and possibly people relatively new to RTS as well.
I don't care if Normal and Hard are designed to provide a challenge even to people with a lot more talent/practice, but 'Easy' simply shouldn't be unbeatable for a player like me, and currently it is.

Apart from those 2 issues the mission seemed fine (played it through with the help of some modded supertanks), so 👍 after these issues are adressed.

@abc013

abc013 approved these changes Dec 18, 2017

The easy difficulty is now much easier :P.

@abcdefg30

What do you need to different players for? Why isn't just one Atreides player fitting enough? If it's just for defending and reinforcing, we should rather fix that, imho.

I'm also not quite happy with the description yet. Kind of confusing where you have to go with your units.

Show outdated Hide outdated mods/d2k/maps/harkonnen-07/harkonnen07.lua Outdated
Show outdated Hide outdated mods/d2k/maps/harkonnen-07/harkonnen07.lua Outdated
Show outdated Hide outdated mods/d2k/maps/harkonnen-07/harkonnen07.lua Outdated
Show outdated Hide outdated mods/d2k/maps/harkonnen-07/harkonnen07.lua Outdated
Show outdated Hide outdated mods/d2k/maps/harkonnen-07/harkonnen07.lua Outdated
@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Dec 19, 2017

Member

What do you need to different players for? Why isn't just one Atreides player fitting enough? If it's just for defending and reinforcing, we should rather fix that, imho.

There were actually 3 different player in original map, i merged 2. Production is one of the reasons of seperation that comes to my mind, as 2 bases should build at the same time. If we get Tooltip PR merged, these names won't be seen anyway. A better fix would be seperating codename and visible name of players, so we can just call all of them Atreides.

I'm also not quite happy with the description yet. Kind of confusing where you have to go with your units.

I don't really wanna mess with the briefing more, but we can add a message at the beginning.

Member

MustaphaTR commented Dec 19, 2017

What do you need to different players for? Why isn't just one Atreides player fitting enough? If it's just for defending and reinforcing, we should rather fix that, imho.

There were actually 3 different player in original map, i merged 2. Production is one of the reasons of seperation that comes to my mind, as 2 bases should build at the same time. If we get Tooltip PR merged, these names won't be seen anyway. A better fix would be seperating codename and visible name of players, so we can just call all of them Atreides.

I'm also not quite happy with the description yet. Kind of confusing where you have to go with your units.

I don't really wanna mess with the briefing more, but we can add a message at the beginning.

@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Dec 19, 2017

Member

Updated.

Member

MustaphaTR commented Dec 19, 2017

Updated.

@reaperrr reaperrr merged commit 956dbb7 into OpenRA:bleed Dec 21, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr
Contributor

reaperrr commented Dec 21, 2017

@MustaphaTR MustaphaTR deleted the MustaphaTR:d2k-harkonnen-7 branch Dec 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment