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

Overhaul RA installed content. #11545

Merged
merged 7 commits into from Jul 30, 2016
Merged

Conversation

pchote
Copy link
Member

@pchote pchote commented Jun 29, 2016

This overhauls the RA asset extraction and downloads with a few goals in mind:

  • Aftermath and C&C Desert assets are now managed by the content installer system. This removes a bunch of binary assets from the repo and cuts almost 1MB from the windows installer download size (according to Appveyor).
  • Downloaded asset packages provide identical content to the data installed from disc.
  • lores.mix gives us a full set of low resolution icons for beacons and the observer interface.

Depends on #11536.
Closes #11537.

@pchote pchote changed the title Aftermath assets Overhaul RA installed content. Jun 29, 2016
@pchote
Copy link
Member Author

pchote commented Jun 30, 2016

backwards compatibility (new playtest <-> last release and new bleed <-> older bleed) is going to be a problem here, and the migrations code won't be able to help. I think our only viable option will be to namespace the assets by putting them in a different directory.

@obrakmann
Copy link
Contributor

In Ruby On Rails, migrations used to have an up() and a down() method, for up- and downgrading respectively, obviously. Would that help here? We would need a way to store the versions of mods that were last loaded/installed, though. But I think that would be helpful in the long run, anyway, once we allow in-game mod installation where people can jump back and forth between mod versions with just a couple of mouse clicks.

@pchote
Copy link
Member Author

pchote commented Jul 4, 2016

I don't think that's worth the effort, and honestly would probably give up on finishing this rework if that became a requirement. The main reason this sucks so badly is that our old package setup is full of years of hacks and technical debt. Once everyone migrates to a consistent and non-mangled set of packages then future changes should be limited to simple additions (if we need assets in mix files that aren't included).

@obrakmann
Copy link
Contributor

No, it's cool, I'm fine with an "upgrades only"-policy. The mod browser should enforce that then, though, otherwise we'd probably get flooded with bug reports.

@pchote
Copy link
Member Author

pchote commented Jul 4, 2016

Yup, that's why I added the changes requested tag. I'm going to wait for #11536 before I touch this again.

@pchote
Copy link
Member Author

pchote commented Jul 30, 2016

Updated.

This now fixes #11665 by completely removing the now-redundant migrations feature.

I have split #11741 to its own ticket because addressing it here would add several hundred more lines to mod.yaml

@@ -240,10 +240,12 @@ ModContent:
TestFiles: ^Content/ra/v2/expand/expand2.mix, ^Content/ra/v2/expand/hires1.mix, ^Content/ra/v2/expand/lores1.mix, ^Content/ra/v2/expand/chrotnk1.aud, ^Content/ra/v2/expand/fixit1.aud, ^Content/ra/v2/expand/jburn1.aud, ^Content/ra/v2/expand/jchrge1.aud, ^Content/ra/v2/expand/jcrisp1.aud, ^Content/ra/v2/expand/jdance1.aud, ^Content/ra/v2/expand/jjuice1.aud, ^Content/ra/v2/expand/jjump1.aud, ^Content/ra/v2/expand/jlight1.aud, ^Content/ra/v2/expand/jpower1.aud, ^Content/ra/v2/expand/jshock1.aud, ^Content/ra/v2/expand/jyes1.aud, ^Content/ra/v2/expand/madchrg2.aud, ^Content/ra/v2/expand/madexplo.aud, ^Content/ra/v2/expand/mboss1.aud, ^Content/ra/v2/expand/mhear1.aud, ^Content/ra/v2/expand/mhotdig1.aud, ^Content/ra/v2/expand/mhowdy1.aud, ^Content/ra/v2/expand/mhuh1.aud, ^Content/ra/v2/expand/mlaff1.aud, ^Content/ra/v2/expand/mrise1.aud, ^Content/ra/v2/expand/mwrench1.aud, ^Content/ra/v2/expand/myeehaw1.aud, ^Content/ra/v2/expand/myes1.aud
Sources: aftermath, aftermath-linux, tfd, ra-origin
Required: true
Download: aftermath
Copy link
Contributor

Choose a reason for hiding this comment

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

Aftermath wasn't released as freeware, was it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not explicitly, no. Nobody seems to care about that fact though and it is far too late for us to remove our dependency on its assets.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, we're shipping it already, this is just changing the mode of delivery, so to speak. Guess we'll have to live with it.

@obrakmann
Copy link
Contributor

Looks fine, 👍

.
$ra: ra
$cnc: cnc
./mods/common: common
~main.mix
~redalert.mix
Copy link
Member

Choose a reason for hiding this comment

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

This makes the game not detect manually copied installations and attempt to download redundant files.

Copy link
Member

Choose a reason for hiding this comment

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

Guess this is a trade-off for better performance. #11741

Copy link
Member Author

Choose a reason for hiding this comment

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

one of the pr goals is to guarantee a standard set of installed assets. Calling this a feature might be a bit far, but it's certainly not an unwanted side effect.

@Mailaender Mailaender merged commit a73163c into OpenRA:bleed Jul 30, 2016
@Mailaender
Copy link
Member

Changelog

@reaperrr
Copy link
Contributor

reaperrr commented Aug 18, 2016

This regressed the RA shellmap slightly, there's now a tile error on the river near the right soviet base.

Probably a missing file that somehow didn't get included in the RA desert content download.

@pchote pchote deleted the aftermath-assets branch August 21, 2016 11:59
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

5 participants