Author Topic: Empires of Eradia: The Cataclysm of Chaos - Alpha V36H6  (Read 43808 times)

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4829
Re: Empires of Eradia: The Cataclysm of Chaos - Alpha V36
« Reply #200 on: September 02, 2019, 11:18:57 AM »
I like that the code is uncompiled now  :)

Offline lordpalandus

  • Hero Member
  • *****
  • Posts: 783
Re: Empires of Eradia: The Cataclysm of Chaos - Alpha V36
« Reply #201 on: September 02, 2019, 12:39:42 PM »
Wasn't the main post always uncompiled? I can't remember.

Anyway, I'm working on a hotfix to address some minor issues in the Tutorial.
Currently working on Cataclysm of Chaos, Remade.
Link to OPU page = http://forum.outpost2.net/index.php/topic,6073.0.html

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4829
Re: Empires of Eradia: The Cataclysm of Chaos - Alpha V36
« Reply #202 on: September 02, 2019, 01:52:53 PM »
I think you had one archive with the original source, back at like V8 or so. After that I remember it being mostly compiled.



Thought I'd mention a common coding pattern, and how it can be refactored to something shorter.

Code: [Select]
if booleanExpression:
  flag = True
else:
  flag = False

This can be shortened to:
Code: [Select]
flag = booleanExpression



For a concrete example from Player.py:
Code: [Select]
def Exploit_Fear(obj):
    if obj.fighter and obj.fighter.feared == True: 
        FearAdvantage = True
    else:
        FearAdvantage = False   
    return FearAdvantage

This can be shortened to:
Code: [Select]
def Exploit_Fear(obj):
    return obj.fighter and obj.fighter.feared


Edit: Looking at the function name Exploit_Fear versus the variable name FearAdvantage, it seems the later may be a better description. The name Exploit_Fear is a verb phrase, but it's a get method, rather than an action with side effects. It may be more clear it name it as FearAdvantage, a noun phrase describing the value being returned.
« Last Edit: September 02, 2019, 02:49:01 PM by Hooman »

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4829
Re: Empires of Eradia: The Cataclysm of Chaos - Alpha V36
« Reply #203 on: September 02, 2019, 03:19:27 PM »
Looking in Inventory.py, another section which can probably be shortened:
Code: [Select]
    if len(List_Equip) > 0:
        for Equipment in List_Equip:

If the length of List_Equip is 0, then the loop should be skipped entirely anyway, so the if doesn't provide any real protection. This can be shortened to just:
Code: [Select]
    for Equipment in List_Equip:


If there is a chance that List_Equip is not an iterable object, then you may want an if guard. However, by testing len(List_Equip) > 0, you're already making an assumption that it is iterable (or at least that it provides a length). Not to mention the variable has "List" in the name, so if it represented anything other than an iterable list, you've got bigger problems.

If for some reason you have a nullable type, which might represent a list, or might represent a null value, then it may make some sense to test first. Same if you have a variant type, where the type can change, so you can't know if any given property will exist. For more on that, you might consider reading:
How to know if an object has an attribute in Python

I would pay particular attention to the discussion of look before you leap versus asking for forgiveness rather than permission. It seems idiomatic Python may be to catch exceptions after the fact, rather than test for the existence of a property before hand.



Here is another example where an if test seems to be making an assumption about the value which it seeks to protect from:
Code: [Select]
            if Equipment.equipment.melee_damage_bonus > 0:
                Melee_Damage += Equipment.equipment.melee_damage_bonus

Presumably not all objects are weapons, and so not all objects will have melee_damage_bonus, and so you're only adding in the bonus for objects of the correct type. However, the if test accesses this field and compares against 0, and so implicit is an assumption that melee_damage_bonus exists as a field, and is a number. Assuming the only problem number is 0, you can just add it anyways, as it won't change the result. If objects that aren't weapons will have melee_damage_bonus set to 0, adding it unconditionally is still correct. I'd recommend just using:
Code: [Select]
            Melee_Damage += Equipment.equipment.melee_damage_bonus

Granted, the results are different here if negative values are allowed. I assume they aren't, or if they were allowed, they maybe should have been added in anyway.


A possible exception to the above, some languages will return a null value for fields that don't exist, rather than throw an error, and some languages can have unexpected comparisons when dealing with certain values, such as infinity, non-a-number (NaN), or null. As an example, if melee_damage_bonus was set to NaN, you might not want to add in the bonus. And as a cool effect, infinity should compare greater than 0, so that will be added in as a bonus.  :)

Offline lordpalandus

  • Hero Member
  • *****
  • Posts: 783
Re: Empires of Eradia: The Cataclysm of Chaos - Alpha V36HH
« Reply #204 on: September 02, 2019, 04:42:27 PM »
Exploit_Fear is only used during combat calculations to check if you have an opposing element (ie to exploit fear, you use cold or shock). The returned FearAdvantage tells the combat code to critically hit the target and then it removes the fear effect after critically hitting them.

A lot of the reason for the rewrite is because coding standards, practices, and methods are different throughout the codebase and the rewrite is meant to bring everything up to spec.

Anyway, released the second hotfix, grabbed here or on the main post:

https://www.mediafire.com/file/fopck6ef5zq5ylq/Binary_V36HH.zip/file
Currently working on Cataclysm of Chaos, Remade.
Link to OPU page = http://forum.outpost2.net/index.php/topic,6073.0.html

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4829
Re: Empires of Eradia: The Cataclysm of Chaos - Alpha V36HH
« Reply #205 on: September 02, 2019, 10:28:46 PM »
Point taken. As the project is a simple archive, there is no version control history, so there's no info on how old any given section of code is. I was probably looking at one of the older sections.

Offline lordpalandus

  • Hero Member
  • *****
  • Posts: 783
Re: Empires of Eradia: The Cataclysm of Chaos - Alpha V36HH
« Reply #206 on: September 03, 2019, 02:01:53 AM »
Well, the Inventory.py is a refactoring of very early code (believe I implemented it in Prototype 5 or 6), which roughly took up about 8 times more lines of code and had a ton of redundancy. It is not to say it couldn't benefit from more work, but from what it was, it is a massive improvement.

Similarly, the exploit fear code was very early stuff, that I haven't touched in over a year. Improvements could definitely be made for that stuff.

But basically, I've learned a lot about coding, refactoring, and designing out good code, so I figure it is time for a rewrite. I'd prefer to keep refactoring it, but there comes a time in development where it is faster to rebuild than to refactor. And as the refactor was looking like it would require ripping out over 60% of the codebase, I figured I should stop putting off the rewrite and just get it over with (as I have known for a while now that I'd probably need to do a rewrite, but I had been putting it off to attempt to refactor it).

Also, not sure I mentioned it anywhere, but I have a Trello board, that I'm using to keep progress on the rewrite for anyone interested in knowing how that is going. I also do updates on my subreddit. I plan on releasing a few tech demos, once the rewrite is a bit farther along to show off some of the new stuff I'm including into the rebuild. I believe you need an account with Trello to view the board, but its not private so you should be able to see it without an invite. Though I don't know as I've not used Trello before.

Trello = https://trello.com/b/orskgQxa/eoe-the-cataclysm-of-chaos
Subreddit = https://www.reddit.com/r/EmpiresOfEradiaCOC/
Currently working on Cataclysm of Chaos, Remade.
Link to OPU page = http://forum.outpost2.net/index.php/topic,6073.0.html

Offline lordpalandus

  • Hero Member
  • *****
  • Posts: 783
Re: Empires of Eradia: The Cataclysm of Chaos - Alpha V36HHH
« Reply #207 on: September 05, 2019, 09:10:22 PM »
More bugs identified by players, so additional hotfix is ready.

Link = https://www.mediafire.com/file/ledx1xm55epfoey/Binary_V36HHH.zip/file
Currently working on Cataclysm of Chaos, Remade.
Link to OPU page = http://forum.outpost2.net/index.php/topic,6073.0.html

Offline lordpalandus

  • Hero Member
  • *****
  • Posts: 783
Re: Empires of Eradia: The Cataclysm of Chaos - Alpha V36H6
« Reply #208 on: September 09, 2019, 07:12:31 PM »
Even more bugs identified by players (and I forgot to upload them here):

https://www.mediafire.com/file/md50ko48v9ic9q6/Binary_V36_Hotfix_6.zip/file

Also includes a potential method of modding the game, for those interested.
Currently working on Cataclysm of Chaos, Remade.
Link to OPU page = http://forum.outpost2.net/index.php/topic,6073.0.html

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4829
Re: Empires of Eradia: The Cataclysm of Chaos - Alpha V36H6
« Reply #209 on: September 18, 2019, 04:50:07 AM »
Sometimes a rewrite may be faster, though I've found refactoring to be such an important skill that I generally tend to try and exercise it anyway. I've also heard that statistically, a rewrite carries a great chance that a project will fail. When doing a rewrite, there tends to be a lengthy period of time when the project either doesn't build, or builds but produces a less complete and inferior seeming program. Something about the psychology of that situation has a way of killing momentum. By refactoring, you maintain a working and complete program, which sidesteps some of the psychological barriers.

Trello is awesome. I used to use it, though at some point let it fall by the wayside.

I've recently learned GitHub has a project board similar to what Trello offers. I haven't really played with it yet though. Apparently it can automatically move cards around as you check in code, based on tags in commit messages. I should really get around to giving it a try.

Offline lordpalandus

  • Hero Member
  • *****
  • Posts: 783
Re: Empires of Eradia: The Cataclysm of Chaos - Alpha V36H6
« Reply #210 on: September 19, 2019, 12:53:06 AM »
Yeah. I've preferred to refactor it in the past for this very reason, but I'm projecting that it will take about 4-8 weeks to refactor the chunk of code (roughly 60% of the codebase has the issue) that desperately needs addressing, and the rewrite would take about 3-6 weeks, to repair all of the codebase. In the past it has been faster to refactor and so I've refactored it, but the seriousness of the problems that I'm currently facing, the rewrite looks faster, and thus far hasn't been an issue yet.

I think the number of projects that fail due to a rewrite is above 50%. So the odds are stacked against me to succeed, but honestly, if I'm willing to put the effort in, then there shouldn't really be any concern. I'm currently bogged down with reading through the libtcod documentation. A bunch of my game's problems is due to a lack of understanding of how to create UIs, UI elements (ie buttons, scrolling, line highlighting, etc...), and how to modify menus. The docs explain hwo to do stuff... not well, but they explain stuff.

The game is fairly stable at the moment. All my duct-tape is keeping things from flying apart. Its only if I try to add new stuff, will it threaten to fly apart. Some bad design decisions, introduced when i didn't know how to code well is what is creating issues. Hence why a rewrite is faster, as I've learned a whole lot more in the past several months.

...

Interesting on Github. I didn't know it has such a feature.
Currently working on Cataclysm of Chaos, Remade.
Link to OPU page = http://forum.outpost2.net/index.php/topic,6073.0.html

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4829
Re: Empires of Eradia: The Cataclysm of Chaos - Alpha V36H6
« Reply #211 on: September 20, 2019, 06:47:42 AM »
Speaking of GitHub and refactoring, the op2ext library has recently been getting a lot of refactoring. An example of one of the smaller branches that was recently completed is:
Update windows error reporting #127

You can click on each of the commits to see a color highlighted diff of each of the committed changes. Aside from one or two commits, most of them are very small and targeted, making them easy to read and understand (well, assuming you know a bit of C++).

If you look closely next to the commit hashes, some of them have green check marks. Those indicate the code was built and passed all units tests. If there was a problem, it'd show a red X instead. Not all commits have a build and unit test run associated with them. The builds and unit test runs can sometimes take a little while, so for faster feedback, the continuous integration servers don't build every commit. They only build the most recent commit after a push.

From the two green check marks you can determine a bit about the pace of development. The first set of commits were done together, and then pushed up to GitHub where the continuous integration servers saw them and did a build. A short while later another few commits were pushed up to GitHub triggering another build and test run.

You can also see some code review comments further down in the thread below the commits. The reviewer can make comments on the code, and can choose to also accept or reject the changes. It's possible to not allow merging of the changes without an approving review, or when there is a rejecting review, though none of the OPU repositories are setup that way. Anyone that's part of the organization can just merge if they feel like it. Waiting for review and following the recommendation is more of a convention.

Towards the very top of the page is a link to a list of Issues, and a list of Pull Requests that are still awaiting review and approval (and sometimes are still being finished).

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4829
Re: Empires of Eradia: The Cataclysm of Chaos - Alpha V36H6
« Reply #212 on: November 02, 2019, 02:55:42 PM »
I was just thinking back to your project today, and it got me wondering what Python has in terms of Continuous Integration support. This got me thinking more specifically about Unit Testing. It seems Python has built-in support with unittest. I thought it might be worth taking a quick look at the "Basic example" section. Having a few unit tests could potentially really help with refactoring.

Offline lordpalandus

  • Hero Member
  • *****
  • Posts: 783
Re: Empires of Eradia: The Cataclysm of Chaos - Alpha V36H6
« Reply #213 on: November 04, 2019, 12:42:17 AM »
Looks like there is also a unittest module for the version of Python I'm using (2.7). I'll give it a read over and figure out how to use it. Thanks for suggesting it. Also looks like I forgot to update the thread here as well. However, as the current uncompiled build is a bit of a mess as it is mid-refactor, I figure I'll just update the thread once V39 is released. V39, will be a major overhaul of a lot of systems, so it will be worth the wait.
Currently working on Cataclysm of Chaos, Remade.
Link to OPU page = http://forum.outpost2.net/index.php/topic,6073.0.html