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

Added Nod Covert Operations mission: scb30ea - Eviction Notice #20660

Merged
merged 1 commit into from
Apr 20, 2023
Merged

Added Nod Covert Operations mission: scb30ea - Eviction Notice #20660

merged 1 commit into from
Apr 20, 2023

Conversation

YAmismo
Copy link
Contributor

@YAmismo YAmismo commented Feb 10, 2023

Hi again, i've been porting this Nod map while learning how to do it. I hope its ready and good enough, at least i had fun doing it.

@abcdefg30
Copy link
Member

Hi. I haven't looked at the changes yet, but there are some lint errors reported by the utility:

OpenRA.Utility(1,1): Warning: The player Neutral owning the world should not have any allies or enemies.
OpenRA.Utility(1,1): Error: rules.yaml:108 refers to a trait field `Prerequisites` that does not exist on `ResourceValueMultiplier`.
OpenRA.Utility(1,1): Error: rules.yaml:112 refers to a trait field `Prerequisites` that does not exist on `ResourceValueMultiplier`.

@YAmismo
Copy link
Contributor Author

YAmismo commented Feb 11, 2023

Can i run this lint test myself on my pc? Just to check before trying to push on every change.

@PunkPun
Copy link
Member

PunkPun commented Feb 11, 2023

Yes, you can run make test in the project directory

@YAmismo
Copy link
Contributor Author

YAmismo commented Feb 11, 2023

Well, test doesn't show any error for me now.

mods/cnc/maps/scb30ea/rules.yaml Outdated Show resolved Hide resolved
mods/cnc/maps/scb30ea/rules.yaml Outdated Show resolved Hide resolved
mods/cnc/maps/scb30ea/scb30ea-AI.lua Outdated Show resolved Hide resolved
mods/cnc/maps/scb30ea/scb30ea-AI.lua Outdated Show resolved Hide resolved
mods/cnc/maps/scb30ea/scb30ea-AI.lua Outdated Show resolved Hide resolved
mods/cnc/maps/scb30ea/scb30ea-AI.lua Outdated Show resolved Hide resolved
mods/cnc/maps/scb30ea/scb30ea-AI.lua Outdated Show resolved Hide resolved
mods/cnc/maps/scb30ea/scb30ea-AI.lua Outdated Show resolved Hide resolved
mods/cnc/maps/scb30ea/scb30ea-AI.lua Outdated Show resolved Hide resolved
mods/cnc/maps/scb30ea/scb30ea-AI.lua Outdated Show resolved Hide resolved
mods/cnc/maps/scb30ea/scb30ea-AI.lua Outdated Show resolved Hide resolved
mods/cnc/maps/scb30ea/scb30ea.lua Outdated Show resolved Hide resolved
mods/cnc/maps/scb30ea/scb30ea.lua Outdated Show resolved Hide resolved
mods/cnc/maps/scb30ea/scb30ea.lua Outdated Show resolved Hide resolved
mods/cnc/maps/scb30ea/scb30ea.lua Outdated Show resolved Hide resolved
mods/cnc/maps/scb30ea/weapons.yaml Outdated Show resolved Hide resolved
Copy link
Member

@abcdefg30 abcdefg30 left a comment

Choose a reason for hiding this comment

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

First observations:

  • There is a single civilian in the village which isn't hostile (no attack cursor) and has the "Unit Lost" notification playing when dying. Is that intentional?
  • The objective to collect all money didn't complete for me
  • Directly sending two mammoth tanks into the base on normal difficulty seems a bit harsh to me. (I assume that is grnf3? It would be nice if those were a bit more descriptive names. I can help with that later though.)

@YAmismo
Copy link
Contributor Author

YAmismo commented Feb 14, 2023

There is a single civilian in the village which isn't hostile (no attack cursor) and has the "Unit Lost" notification playing when dying. Is that intentional?

No, he was there in the original map, simply i didn't change that. Maybe he's there to throw the flare? Like a spy possibly.

The objective to collect all money didn't complete for me

There are 5 crates, did you found it? I managed to build and run the dev_version and it works for me.

Directly sending two mammoth tanks into the base on normal difficulty seems a bit harsh to me. (I assume that is grnf3? It would be nice if those were a bit more descriptive names. I can help with that later though.)

It's grnf1 (i'll change few names like that for more descriptive ones).
The team are triggered by destroying certain civilians buildings, (only 5 of them), so you are right its too random when the player doesn't know why this happens. Maybe taking more buildings to be triggered... or giving a warning while doing so.
Making it trigger by destoyed buildings count (different number on difficulty), instead of specific ones... could work. I'll try this later.

@PunkPun
Copy link
Member

PunkPun commented Feb 22, 2023

Hello, could you squash the commits?

@YAmismo
Copy link
Contributor Author

YAmismo commented Feb 23, 2023

Sorry, can't understand what you mean by squash, my english is far from perfect. Can you explain?
It's ok, just discovered what is it.

@YAmismo
Copy link
Contributor Author

YAmismo commented Feb 23, 2023

Sorry again, i cant figure out how to do it. I'll try again later.

@YAmismo YAmismo closed this Feb 23, 2023
@YAmismo YAmismo reopened this Feb 23, 2023
@abcdefg30
Copy link
Member

Sorry for the long delay in answering.

No, he was there in the original map, simply i didn't change that. Maybe he's there to throw the flare? Like a spy possibly.

I like the idea. We should definitely either do something with the civ or change its owner. As it is now it just feels weird Imo, and gives you a "Unit Lost" notification.

There are 5 crates, did you found it? I managed to build and run the dev_version and it works for me.

My mistake, I didn't notice the crate spawned by the single house in the north.

It's grnf1 (i'll change few names like that for more descriptive ones).
The team are triggered by destroying certain civilians buildings, (only 5 of them), so you are right its too random when the player doesn't know why this happens. Maybe taking more buildings to be triggered... or giving a warning while doing so.
Making it trigger by destoyed buildings count (different number on difficulty), instead of specific ones... could work. I'll try this later.

That works quite good. I still think the mammoths are a bit strong, but now it's not random any more and should be predictable by the player.

@YAmismo
Copy link
Contributor Author

YAmismo commented Feb 23, 2023

Sorry for the long delay in answering.

No problem at all.

Maybe change the civilian for a minigunner? For the flare story. Or better remove it?
What you think?

@YAmismo
Copy link
Contributor Author

YAmismo commented Feb 23, 2023

Replaced the civilian with a minigunner, it's more clear now that a nod unit was there, waiting reinforcements.

@YAmismo
Copy link
Contributor Author

YAmismo commented Feb 27, 2023

Done.

Mailaender
Mailaender previously approved these changes Feb 27, 2023
@penev92
Copy link
Member

penev92 commented Feb 28, 2023

@YAmismo can you confirm which mission this is? #4988 lists scb30ea as "N64 Special Ops - Nod", while "CovertOperations - Nod" are scb21-28 🤔

@YAmismo
Copy link
Contributor Author

YAmismo commented Feb 28, 2023

I extracted (with xcc mixer) and converted scb30ea.ini/bin from sc-001.mix.

According to https://cnc.fandom.com/wiki/Eviction_Notice_(Covert_Operations) and the new c&c remastered, its scb30ea Nod Covert Operations.

Don't know where #4988 list came from, but it does not coincide with any list i can find.

@penev92
Copy link
Member

penev92 commented Feb 28, 2023

OK, thanks. Then we may need to fix #4988

@YAmismo
Copy link
Contributor Author

YAmismo commented Feb 28, 2023

ProdQueue = { i = { }, v = { }, a = { } }
ProductionQueue = { infantry = { }, vehicle = { }, aircraft = { } }

Just fixed a bug i accidentally introduced while renaming this.

Copy link
Contributor

@JovialFeline JovialFeline left a comment

Choose a reason for hiding this comment

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

Hello! This is more of a playtest review than a code review. I went through on Hard and Normal, after completing the original mission for comparison. I thought this was a pretty good translation to ORA, so kudos.

I did find it a bit rough to complete, but this was even more true of the original in my experience, and most of the failures were early game. That's fair enough since it's an expansion mission for a game that's routinely mean. I'll also share that the Ion Cannon zapped one of my turrets and the civilian building next to it. That prompted a warning about wrecking the village, which I found amusing. This is not a critique; no change is needed!

I have not had issues with how the script works and I don't intend to suggest large changes. However, I do have suggestions for easy polish if you can indulge me.

#20743 will get a look from me once I can manage the original. The early game has been a doozy, and then I learned Nod has nukes on that map. Huzzah.

mods/cnc/languages/lua/en.ftl Outdated Show resolved Hide resolved
mods/cnc/languages/lua/en.ftl Outdated Show resolved Hide resolved
mods/cnc/languages/lua/en.ftl Outdated Show resolved Hide resolved
mods/cnc/languages/lua/en.ftl Outdated Show resolved Hide resolved
mods/cnc/maps/scb30ea/scb30ea.lua Outdated Show resolved Hide resolved
mods/cnc/maps/scb30ea/scb30ea-AI.lua Outdated Show resolved Hide resolved
mods/cnc/maps/scb30ea/scb30ea-AI.lua Outdated Show resolved Hide resolved
mods/cnc/maps/scb30ea/scb30ea-AI.lua Outdated Show resolved Hide resolved
mods/cnc/maps/scb30ea/scb30ea-AI.lua Outdated Show resolved Hide resolved
mods/cnc/maps/scb30ea/scb30ea.lua Outdated Show resolved Hide resolved
@YAmismo
Copy link
Contributor Author

YAmismo commented Mar 27, 2023

Nice review, thanks. I'll take a look on all of this.
Those translation issues... haha my english still needs a lot of polish.

@YAmismo
Copy link
Contributor Author

YAmismo commented Mar 27, 2023

Updated with suggested changes by JovialFeline, renamed map files and capture structures function improved.

JovialFeline
JovialFeline previously approved these changes Apr 4, 2023
Copy link
Member

@Smittytron Smittytron left a comment

Choose a reason for hiding this comment

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

Code LGTM sans a few spacing nits.

mods/cnc/maps/eviction-notice/eviction-notice.lua Outdated Show resolved Hide resolved
mods/cnc/maps/eviction-notice/eviction-notice.lua Outdated Show resolved Hide resolved
@YAmismo
Copy link
Contributor Author

YAmismo commented Apr 19, 2023

Spaces removed 😉

@Smittytron Smittytron merged commit 8b52268 into OpenRA:bleed Apr 20, 2023
@Smittytron
Copy link
Member

Changelog

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

7 participants