search

vaendryl / Sunrider

source code of the visual novel 'Sunrider'
http://sunrider-vn.com/
41 stars 21 forks source link

Insane commit | Battle management improvement #35

Closed DoumanAsh closed 10 years ago

DoumanAsh commented 10 years ago

Content:

  1. IMP: Small style updates
  2. Getting rid of compassion string>int
  3. IMP: classes.rpy - move main part of battle() code to separate functions and introduction of battle dispatcher

I had problem with running battles due to missing pics for skirmishes.I'm using 4.2 as base :D Not sure why though, i haven't touched it in my commit? Anyway this commit looks insane with all these diffs...

In case if such serious changes ain't acceptable i can just separate small changes(1 and 2) from this freaking big one(3) I made 3 change due to just a way too big battle() function. I guess it's going to become even more bigger. Not sure if it will lead to any improvements though...

EnderShadow commented 10 years ago

I'll leave this to Vaendryl.

vaendryl commented 10 years ago

I agree putting the various if result=='something' into a dictionary is far more pythonic and is the preferred way to emulate the common 'case' statement. the current nonsense is an artifact from way back when when I first started building a combat engine without the tinies clue as to what I was doing. however, I hold a great dislike for CamelCasing anything but classes >.>

also, it says here (http://legacy.python.org/dev/peps/pep-0257/) that docstrings should be in triple quotes so I'm trying to move towards that when making new functions *shrug

DoumanAsh commented 10 years ago

Regarding camelCasing. This is my habit from work due to our design rules. One of them states that function name starts with lower case(but each word in name should started with upper case) Anyway it's not a problem for me just put some another style for dispatcher functions. Like dispatch_something()

Regarding docstrings. As i understand these are pure strings which are intended for documentation? Are you planning to make one? :) Do i need to revert these strings back?

Also please note that i changed handling of results for battles so they always should lists. It makes easier to make simple common dispatcher for all ui events

DoumanAsh commented 10 years ago

It's back... It was a very difficult to move formation and skirmish parts but it seems i made it fine... I hope...

vaendryl commented 10 years ago

damn, you've been busy.

I'll have to do quite a bit of testing before I merge this though. I'll leave it open for the moment. fixin' what ain't broke is a dangerous thing to do.

DoumanAsh commented 10 years ago

Thanks for review, Ender Vaendryl, sorry about size. Do i need to merge all in one commit or it will be fine like that?

EnderShadow commented 10 years ago

Sorry for the comment spam. I was doing it on my phone so I couldn't see line numbers.

DoumanAsh commented 10 years ago

Also first two commits isn't ok at all. Only together with last 3 they should be ok :)

DoumanAsh commented 10 years ago

If will be needed i can rebase these commits over current master

DoumanAsh commented 10 years ago

Everything is merged to 1 commit and rebased on latest changes

DoumanAsh commented 10 years ago

Abond it, no point to keep it like that