Empires of Eradia: The Cataclysm of Chaos - Alpha V48H3

[Hooman]

I like that the code is uncompiled now 

[lordpalandus]

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.

[Hooman]

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.

[Hooman]

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. 

[lordpalandus]

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

[Hooman]

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.

[lordpalandus]

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/

[lordpalandus]

More bugs identified by players, so additional hotfix is ready.

Link = https://www.mediafire.com/file/ledx1xm55epfoey/Binary_V36HHH.zip/file

[lordpalandus]

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.

[Hooman]

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.

[lordpalandus]

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.

[Hooman]

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).

[Hooman]

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.

[lordpalandus]

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.

[lordpalandus]

After many months of development hell, dodging burnout and depression, here is a new build.

It has a new form of changelog and known issues, allowing you to view them in their entirety from within the game itself.

Link = https://www.mediafire.com/file/dmbpqthbm6cr0jw/Binary_V39.zip/file

[lordpalandus]

Released a Hotfix addressing a few issues raised by some players.

Link = https://www.mediafire.com/file/8m17oj647bk5u3e/Binary_V39H1.zip/file

[lordpalandus]

Released a second hotfix, fixing some more typos and some more crash bugs.

Link = https://www.mediafire.com/file/g8pi4ani3uy0qx4/Binary_V39_H2.zip/file

[Hooman]

You know, I was actually thinking of asking about this project right before you posted. I didn't get around to downloading the update until just now though.



As per usual, I like to peruse the source code, and see if there is anything that can be tweaked. The following method caught my eye:

Code: [Select]

def Distance_To_Object(Me, You):  
    '''This function determines the distance between two objects and. makes negative values positive.
    It then determines which value is higher, and returns that value to the calling function.'''
    X_Value = Me.x - You.x  
    if X_Value < 0:
        X_Value = X_Value * -1
    Y_Value = Me.y - You.y  
    if Y_Value < 0:
        Y_Value = Y_Value * -1
        
    if X_Value > Y_Value:
        return X_Value
    else: #less than or equal.
        return Y_Value

First off, I noticed the "X_Value * -1", which seems very mathematical, and something I've sometimes been tempted to write. It can of course be simplified using unary minus "-X_Value":

Code: [Select]

X_Value = -X_Value

Of course, each if block is just performing absolute value:

Code: [Select]

if X_Value < 0:
        X_Value = -X_Value

There is an absolute value function:

Code: [Select]

X_Value = abs(X_Value)

That can of course be combined with the subtraction expression, resulting in more simplified code:

Code: [Select]

X_Value = abs(Me.x - You.x)
Y_Value = abs(Me.y - You.y)

The next if block is then determining the max value:

Code: [Select]

    if X_Value > Y_Value:
        return X_Value
    else: #less than or equal.
        return Y_Value

There is also a function for this common operation. It could become:

Code: [Select]

return max([X_Value, Y_Value])

Of course you can inline expressions, so the function becomes:

Code: [Select]

def Distance(obj1, obj2):
    return max([abs(obj1.x - obj2.x), abs(obj1.y - obj2.y)])

But wait, there's more!

If you were to represent positions as compound (x, y) objects, rather than as individual components x and y, you could potentially do vectorized operations, which work on all components simultaneously (from a programming interface perspective, more so than a computational hardware persepctive). In particular, you're doing the 2D case of: Point - Point = Vector

Imagine instead of individual x and y components, you had a single object representing the compound value (x, y), and imagine that it had overloaded subtraction operations. You would then be able to write code such as:

Code: [Select]

obj1.position - obj2.position

That could effectively represent a conceptual vectorized operation:

Code: [Select]

# Not valid syntax
(obj1.position.x - obj2.position.x, obj1.position.y - obj2.position.y)

Since it's very common to operate on both x and y components together, and in the same way, this can save a lot of typing over a large code base.

If the resulting Vector result was also an object, it might provide a length method representing your chosen distance metric. In particular, the maximum of the difference between x and y values. The function could then be written as:

Code: [Select]

def Distance(obj1, obj2):
    return (obj1.position - obj2.position).length

I haven't done much work with Python, but a quick Google search shows it is capable of operator overloading:
Operator Overloading in Python

The Complex number class looks very similar to the code you would need for a Point (position) or Vector (displacement) class.

[lordpalandus]

1) I didn't know that I could just do:

X_Value = -X_Value

2) I don't use absolute value function because I'm trying to make this function as highly performant as possible. It is used in three main chunks of code (its used elsewhere, but these are the big ones): the renderer for gradient shadow effect, enemy pathfinding when A* isn't used, and targeting foes with AOEs. As such, this particular function is called anywhere from 1000 to 3000+ times per turn. From what I understand, absolute value takes more ram and cycles than multiplication, so I use multiplication. Yes it may be peanuts in terms of differences in today's limits, but I have to keep in mind that not everyone has a powerful rig like I do, that may be playing the game. I am trying to address other bottlenecks that are more obvious a problem, but as this one does get called a lot each round, I wanted as good performance as I could get out of it. Before I made this function call, I was using square root to calculate distance, which was then converting the value from a decimal to an integer.

3) I forgot about the Max function, when designing this function. Thanks for the reminder.

4) I try to avoid inline statements as I find it harder to read, and figure out what it does. I have this function and other functions in that file as commonly used functions. When I visit that file looking for an ideal general-case common function I want to be able to look at it quickly and pick the ideal one out of the bunch. Or build a new one, if one isn't present.

EDIT:

Don't I have a function in that file, that does what you discuss? Like this function:

Code: [Select]

def Distance_To_Coordinates(Me, X, Y):
    X_Value = Me.x - X 
    if X_Value < 0:
        X_Value = X_Value * -1
    Y_Value = Me.y - Y 
    if Y_Value < 0:
        Y_Value = Y_Value * -1
        
    if X_Value > Y_Value:
        return X_Value
    else: #less than or equal.
        return Y_Value   

It uses just the object's x,y coords with submitted x,y coords provided for it. I believe this function is used by the Treasure Fiend's Blink ability, which teleports it to a random point on the map.

Also why is that not valid syntax? With class composition, it should work...

obj1 is the owner class, position is a class member variable and x/y is a member variable of the position class.

I might be able to convert the obj.x = a and obj.y = b to obj.position = (x, y). The complexity however is the Tile class and how the map is stored into memory. How it works is it creates a nested list comprehension of 0 to map width and 0 to map height, and makes each x and y, an instanced object of class Tile. For example:

Code: [Select]

    LIST_Map = [[ Tile(True)
             for y in range(MAP_HEIGHT) ]
           for x in range(MAP_WIDTH) ]  

Thus, when it checks for blocked tiles, it checks in this fashion:

Code: [Select]

    LIST_Map[x][y].blocked     #with .blocked a member variable of class Tile

So, I tried to change it from a list, to a dictionary of x,y coordinates, and the libtcod library really, doesn't like it and refused to display the map. Some internal, poorly blackboxed function requires it to be stored in a list, in this fashion of list_map[ x ][y], otherwise the map fails to be placed on the screen. Not sure why, and don't have access to the C source, so I cannot really say why it does this.

There is a lot of things about libtcod that I don't like, as I spend more time working on it. So, I'm doing what I can to phase in improvements now with each build, and clean up the codebase as I go along. Right now, V40, is in the process of rebuilding and updating the XP Calculating and Awarding systems.

[Hooman]

Yes, the code I initially presented was pre-existing, taken from your project. I was giving an example of how to shorten it.

As for the supposed invalid syntax, you're right, that does seem to be valid syntax for Python. Though with the slight caveat that it produces a Tuple, rather than another Point object.

As for the abs() function, it's a very cheap function to call. There's a good chance it is implemented as a simple conditional, much like you already had. That's often how I see it implemented in standard libraries for various languages. Actually, when translated to assembly code, it can be done even without a conditional branch. Having branch free code can help prevent pipeline stalls. A common instruction sequence uses the 2's complement identity where flipping the bits and adding 1 produces the negative of the previous value. The typical absolute value sequence I've seen in Outpost 2 code consists of 3 assembly language instructions:

Code: [Select]

cdq  ; Sign extend eax into edx  (convert double word to quad word) (edx becomes 0 if eax is positive, and -1 if eax was negative)
xor eax, edx  ; (conditionally flip bits: eax is unchanged when edx is 0 (eax is positive), and flipped when edx is -1)
sub eax, edx  ; (conditionally add 1: eax is unchanged when edx is 0 (eax is positive), and adds 1 when edx is -1)

All 3 instructions are very cheap, and with low latency. In contrast, a multiply instruction, while still being reasonably cheap, tends to have higher latency. The end result is these 3 instructions are likely comparable in executable time to a single multiply instruction, and the multiply would need to be somehow made conditional, so you'd need an extra instruction or two to handle that.

Now if you convert inline if block code to a call sequence, there may be overhead from the call sequence, so I don't know for certain if abs() of an if block would be faster. Though the difference would be so small, it would be hard to measure. You'd probably need millions of iterations to reliably measure a difference, and even then, the difference may be lost in statistical variations. Plus, given the nature of Python being an interpreted language, it could be the abs() function is highly optimized, or inlined, and perhaps executes faster than a compound if statement could be interpreted. It's hard to say without measuring it. Very likely though, there won't be a significant difference either way.

I would encourage you to measure the difference if you're concerned about speed matters. The results might be interesting.


There will be a different memory layout between a list and a dictionary. If libtcod is coded in C, then it likely uses a fixed memory layout, and so you'd need to pass values that match that expectation. That may be why it requires that data in list format.

Anyway, I'm not too certain what the difficulty is concerning LIST_map. If the x and y values were packed into a single object, you can still unpack the values manually where needed (and generally only deep in the API, in very few places):

Code: [Select]

LIST_Map[position.x][position.y].blocked

[lordpalandus]

Ok good to know. Thanks for the indepth reply.

EDIT:
Also, released a hotfix for V40, making the tutorial completeable again (V39 apparently broke a whole lot of the tasks).

Link = https://www.mediafire.com/file/hjecrau6kf939yb/Binary_V40H1.zip/file

[lordpalandus]

Finished work on V41 for anyone interested. Link = https://www.mediafire.com/file/8fcvcslaea7qrlh/Binary_V41.zip/file

This release focused on improving the UI significantly in a lot of different areas and added a bunch of lore to the game, found in external text files (for better readability, until I can figure out a more permanent solution).

[lordpalandus]

After a long trial of bugs, delays, and headaches, V43 is finally ready for consumption.

I'll be posting a 3-month update on Roguelikes, and I'll update this post when that is ready (as well as the main page, as usual).

Link = http://www.mediafire.com/file/fvl1v3eh4h73mh7/Binary+V43.zip/file

EDIT = 3 Month Update = https://www.reddit.com/r/roguelikes/comments/ip0lde/empires_of_eradia_the_cataclysm_of_chaos_3_month/

[lordpalandus]

Got a lot of good feedback, recently, and pushed out a hotfix release fixing a lot of things.

Biggest change is I found a pair of very readable, rectangular, Terminal fonts, that has improved the game drastically. Defaults to 8x12, but a 10x16 is available.

Link = http://www.mediafire.com/file/6cr9zer14et2bx7/Binary+V43H.zip/file

[lordpalandus]

Got some more good feedback, and pushed out the second hotfix.

Link = http://www.mediafire.com/file/ggd1fnxyb68v9yh/Binary+V43H2.zip/file

Fixes a rare crash bug, adds some new fonts, and severely reduces the amount of text in most menus, making them much cleaner and easier to use.