luxeengine / alpha

alpha - deprecated 2015~2016. unrelated to the new engine! view the new engine here - https://luxeengine.com/
MIT License
565 stars 74 forks source link

ParcelProgress; Fixing null object exception when no_visuals=true #349

Closed st3veV closed 8 years ago

ruby0x1 commented 8 years ago

Thanks for the PR! It needs a few minor tweaks though.

We still want set_progress to be called, because the user typically overrides the base class for convenience, and uses that function update it's own visuals. The fix needs to be around the direct cause: the set_progress function is touching the default visual element progress_bar which is what needs to be shielded instead.

Some small but important notes: The code should follow the consistency of the existing code, braces on the same line, indented with spaces instead of tabs, things like that :) You may also want to PR from a branch in future, since the PR will track the branch and if you need to make other fixes, they'll accidentally be roped into your existing PR here.

Thanks!

st3veV commented 8 years ago

Well... if the flag is set to no_visuals=true then I wouldn't expect anyone to try updating any visuals :) but okay, I'm going to move it closer to the progress_bar where it makes more sense.

Note to self: official GitHub windows application is not showing whitespace characters, Tortoise is better :)

ruby0x1 commented 8 years ago

Yea, no_visuals is a flag for the default visuals to not be created (i.e "I want to do my own visuals in a subclass") - If you have no visuals at all, you wouldn't need this parcel class at all :+1: The purpose is a loading screen - the default state is a simple minimal placeholder, and the common use case is to extend it and fill in some visual customization.

The parcel itself sends all the events already, this class is a convenience for a quick and easy loading screen that you can hook into :)

st3veV commented 8 years ago

If you have no visuals at all, you wouldn't need this parcel class at all

Fair point

ruby0x1 commented 8 years ago

I just squashed the commits into one clean one, it keeps the contributor though. merged via https://github.com/underscorediscovery/luxe/pull/353

Thanks!