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

Background Changing Framework #4503

Merged
merged 24 commits into from
Sep 2, 2019
Merged

Background Changing Framework #4503

merged 24 commits into from
Sep 2, 2019

Conversation

multimokia
Copy link
Member

Introducing MASBackgrounds:

BG changing framework is now here with this, which introduces MASBackground objects

  • These objects have a variety of parameters which handle the day/night image to use as the 'room', and optional parameters which are for how the 'room' should look while it's raining, snowing, or overcast. (See the init for a MASBackground object for details)
  • They also have the parameters to either hide_masks (so we don't have weather at all) and hide_calendar, which will not display the calendar for the drawing of the spaceroom.
  • Backgrounds are also unlockable which allows for future expansion.

Other changes:

  • Weather objects now have a precip_type associated with them. This allows us to decide what room to use (def, rain, overcast, or snow)
  • As a result of allowing potentially different weather to be a case, window masks had to be changed to full size, so any possible future rooms would be supported by our current weather system. (Thank you @Legendkiller21 for helping out in making these, along with @Velius94 and @Orcaramelo for the frost on the snow windows + the night/overcast/space clouds)
  • There is also a selector for BGs, but as we do not have any other rooms to use, it's init is commented out. (Feel free to review it for now, but I'd probably save it until we get backgrounds to use it for.)
  • Also added the new night bg

Testing:

  • Please make sure everything looks good
  • Ensure that spaceroom calls also work right (shouldn't be as bad as last time since not that much changed)
  • Ensure that progressive weather adjusts the room correctly

@multimokia multimokia added awaiting testing code needs to be tested awaiting code review someone needs to check for syntax/logic/indentation errors labels Jul 27, 2019
@@ -741,7 +724,7 @@ init 1 python:
# NOTE: this must be a string
# NOTE: if passed in, this will override aff-based exps from dissolving.
# (Default: None)
label spaceroom(start_bg=None, hide_mask=False, hide_monika=False, dissolve_all=False, dissolve_masks=False, scene_change=False, force_exp=None):
label spaceroom(start_bg=None, hide_mask=store.mas_current_background.hide_masks, hide_monika=False, dissolve_all=False, dissolve_masks=False, scene_change=False, force_exp=None, hide_calendar=store.mas_current_background.hide_calendar, day_bg=store.mas_current_background.getDayRoom(), night_bg=store.mas_current_background.getNightRoom()):
Copy link
Member

Choose a reason for hiding this comment

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

python named args like these are resolved on init and don't change dynamically, so if the current bg changes these params wont update. Even if renpy actualyl does this dynamically, should just use None for dynamic cases so no one looks at this and tries to do the same in py.

So have these default to None, then after the with None, check those args for None values and set to defaults accordingly.

@ThePotatoGuy ThePotatoGuy added the awaiting changes the pull request creator needs to make some changes label Aug 17, 2019
if image_rain_day:
self.image_rain_day = image_rain_day
else:
self.image_rain_day = image_day
Copy link
Member

Choose a reason for hiding this comment

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

since you repeat this pattern of if statements, i would make a private function like:

def __handleBG(self, desired_bg, fallback_bg):
    if desired_bg:
        return desired_bg
    return fallback_bg

then you can use that here like:

self.image_rain_day = self.__handle_bg(image_rain_day, image_day)

return self.image_rain_day
elif weather.precip_type == mas_weather.PRECIP_TYPE_OVERCAST:
return self.image_overcast_day
elif weather.precip_type == mas_weather.PRECIP_TYPE_SNOW:
Copy link
Member

Choose a reason for hiding this comment

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

seeing that this pattern is being used (if type == constant, return value), it would be a lot nicer to use a dict mapping the constant to its appropriate image. Probably could even combine the day and night elements together like:

img_map = {
    PRECIP_TYPE_RAIN: (day_image, night_image),
    ...
}

Monika After Story/game/zz_backgrounds.rpy Show resolved Hide resolved
Monika After Story/game/zz_backgrounds.rpy Show resolved Hide resolved
for mbg_id, mbg_obj in mas_background.BACKGROUND_MAP.iteritems()
if mbg_obj.unlocked
]
) < 2:
Copy link
Member

Choose a reason for hiding this comment

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

i guess you're trying for brevity here, but this does a useless list build. I think you should make this simpler i.e: like

unlocked_count = 0
for mbj_obj in mas_background.BACKGROUND_MAP.itervalues():
    unlocked_count += int(mbg_obj.unlocked)

if unlocked_count < 2:
    # etc

and since this is in a global store, then it would be actually better to make that for loop in a function in the mas_background store where it returns the unlocked count, then you can do like:

if mas_background.unlockedBG_count() < 2:
    # etc

Monika After Story/game/zz_backgrounds.rpy Show resolved Hide resolved
Monika After Story/game/zz_backgrounds.rpy Show resolved Hide resolved
Co-Authored-By: ThePotatoGuy <[email protected]>
ThePotatoGuy
ThePotatoGuy previously approved these changes Sep 1, 2019
@ThePotatoGuy ThePotatoGuy added DONT MERGE srs and removed awaiting changes the pull request creator needs to make some changes awaiting code review someone needs to check for syntax/logic/indentation errors labels Sep 1, 2019
@ThePotatoGuy
Copy link
Member

waiting for #4582

Copy link
Member

@jmwall24 jmwall24 left a comment

Choose a reason for hiding this comment

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

Everything works, just the calendar glows a bit at night against the new night filter, so if that could darken when we go to night, that'd be great.

@jmwall24 jmwall24 added the awaiting changes the pull request creator needs to make some changes label Sep 1, 2019
@jmwall24 jmwall24 removed the awaiting changes the pull request creator needs to make some changes label Sep 1, 2019
@ThePotatoGuy ThePotatoGuy removed the awaiting testing code needs to be tested label Sep 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants