search

XelaPy / PyTFall

Open World H/Sim Game:
http://www.pinkpetal.org/
23 stars 24 forks source link

refactored logic of image path creation #335

Closed pionere closed 5 years ago

pionere commented 5 years ago

please have a look at the comments (FIXME, TODO) as well

XelaPy commented 5 years ago

I don't like this, f*cking around with this method is not a good idea. Small changes in the past just resulted in endless debates and vague bug-hunts.

Things we can keep: 'choice' instead of 'sample' probably matters not for small sets we're working with here but whatever... 'choice' looks better at the very least :) Adding 'type' is a very good idea. # FIXME regardless of type ??? This is a minor oversight in the original setup.

Otherwise, refactor raises more questions than it gives answers and this is possibly very counterproductive cause trying to figure out all of the changes in the code that's been working for us in the last 2 or 3 years is very time-consuming. Unless you can guarantee that behavior is unchanged or improved with the new code, figuring it out will just interfere with other areas we're trying to develop atm.

pionere commented 5 years ago

I can change it in a way that it is guaranteed that the behavior is unchanged or improved with the new code. There are two behavioral changes: one is the part near the comment '# if len(tags) == 1 ...' where the mood tag was ignored. So if the lines 671-672 are commented out, then the behavior is exactly the same as before. The other one is the shuffle part, which can be added again, but I'm not sure if that is really useful.

But I agree that this is quite a refactor, that is why I did not commit it to the trunk directly.

I'm more confident that the changes are safe (especially reverting the above mentioned two changes), than in my ability to add the 'type' filter ;)