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

Rework shellmap UX. #12465

Merged
merged 5 commits into from
Dec 23, 2016
Merged

Rework shellmap UX. #12465

merged 5 commits into from
Dec 23, 2016

Conversation

pchote
Copy link
Member

@pchote pchote commented Dec 10, 2016

This fixes a bunch of long-ignored annoyances with our shellmaps.

Mute the shellmap activity for all mods.

This one is a no-brainer that has been talked about for ages. This also disables the world sounds while the ingame menu is active, as suggested in #11355.

Closes #5619.
Closes #10700.
Closes #11355.
Closes #12299.

Remove the disable shellmap option.

I expect this will be a bit controversial, but the main reason for this toggle to exist was to mute the shellmap. Removing it cleans up the hacky code that implemented it, and puts an end to the bugs that keep on sneaking in for people with it disabled.

Closes #4966.
Closes #10762.

Replace the TD shellmap with the "shellmap disabled" background.

I expect this will also be quite controversial, but this is another case where a long-unfinished feature has sat rotting for years with no progress. IMO this background is more polished than the unfinished shellmap, and it serves as an example for modders who might want to do a more elaborate non-map menu background.

Closes #3016.
Closes #6943.

@obrakmann
Copy link
Contributor

For some reason it doesn't work on the d2k shellmap, you can still hear sounds. The changes look correct, though.

@pchote
Copy link
Member Author

pchote commented Dec 10, 2016

Works for me, so i'm not sure what to suggest.

@@ -84,6 +87,9 @@ public MusicPlaylist(World world, MusicPlaylistInfo info)
CurrentSongIsBackground = false;
}

if (info.DisableWorldSounds)
Game.Sound.DisableWorldSounds = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't work in d2k for me and @obrakmann because we don't have any d2k music installed, so the return in line 62 prevents the code from ever getting here.

I guess you just need to move this check to the beginning of the constructor.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice catch!

@pchote
Copy link
Member Author

pchote commented Dec 11, 2016

Fixed.

Copy link
Contributor

@reaperrr reaperrr left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@@ -103,7 +103,7 @@ public void DeployTransform(bool queued)
// Only play the "Cannot deploy here" audio
// for non-queued orders
foreach (var s in info.NoTransformSounds)
Game.Sound.PlayToPlayer(self.Owner, s);
Game.Sound.PlayToPlayer(SoundType.World, self.Owner, s);
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be UI, I think.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nevermind, I just noticed the line below.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep. The way these are used in the mods are a giant mess that needs to be rewritten. For now, i've assigned the type based on how they are used in the default mods (e.g. all the support power stuff plays UI sounds, most other things play world sounds).

Copy link
Contributor

Choose a reason for hiding this comment

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

Notifications on UI and sounds for world always seemed like the intention to me, so I fail to see the giant mess. There are some cases where they mixture, like at crates though, but again those looks like legacy edge-cases to me..

Copy link
Member Author

Choose a reason for hiding this comment

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

some → quite a few = mess.

Copy link
Contributor

@obrakmann obrakmann left a comment

Choose a reason for hiding this comment

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

👍 . Needs a rebase, though.

@pchote
Copy link
Member Author

pchote commented Dec 23, 2016

Rebased.

@obrakmann obrakmann merged commit f35c83e into OpenRA:bleed Dec 23, 2016
@obrakmann
Copy link
Contributor

Changelog

@hypercube33
Copy link
Contributor

Um, what the hell? Why was this disabled again?

@Nova900
Copy link

Nova900 commented Feb 5, 2017

We should still have an option to disable the shellmap. Maybe one with better code or whatever, but people should not be forced to have the shellmap playing.
I don't know why you want to disable the C&C shellmap. It looks pretty nice, but I'm okay with that change.

@Micr0Bit
Copy link
Member

Micr0Bit commented Mar 30, 2017

this forces every machine to run a quite ressource-hungry (even without sounds) background when waiting for a game to start ... we shouldnt have this option removed

and i expect a bigger backlash here ... once the release is out

here is difference on my AMD Athlon(tm) II P320 Dual-Core 2.10 ghz / ATI Radeon Mobile 5000 series (sound turned off) ... no need to say that it gets worse with time the longer the shellmap is running ...

off
off

on
on

@gdavegdave
Copy link
Contributor

Can we have the option to disable the shellmap again please? As @Micr0Bit said, it is resource hungry (it actually seems worse now than it used to be!). I was in an RA lobby with American Blunt, who usually has good ping but was yellow due to the shellmap and complained of the inability to disable it. I too would usually disable it, primarily because of the sound, but also because of the needless resource usage and the distraction.

@Micr0Bit
Copy link
Member

Micr0Bit commented Apr 23, 2017

@gdavegdave : for the current release , a blank-shellmap will help
http://www.sleipnirstuff.com/forum/viewtopic.php?f=82&t=20080

#13188

@gdavegdave
Copy link
Contributor

Thanks @Micr0Bit, that works. I've seen a couple of players asking about disabling the shell map (one in Global and one directly to me in Lobby, having seen this thread), so it does seem to be an issue for some.

@schpaul
Copy link

schpaul commented Jan 18, 2018

@dragunoff: Thank you for information.

Is it complicated to implement a switch (shell map on/off) in the settings menu?

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