delete this post (moved)

Started by XeoNovaDan, March 03, 2017, 06:17:00 PM

Previous topic - Next topic

XeoNovaDan


b0rsuk

Ewww, an .exe ! That's not Python. I guess I'll have to write my own.

I could run a Python program on Linux with no problem.

XeoNovaDan

Ah, I already complied it using pyinstaller. I could add the source code as a separate download?

GiantSpaceHamster

Are you aware that you can mouse-over a target in-game to see your modified chance to hit? Same goes when selecting enemies and hovering over colonists. There's also a mod called "Numbers" that provides shooting accuracy, aim time, and melee DPS values in-game.

It's not entirely clear to me if this is the same info that your program provides, just wondering if you're re-inventing the wheel or not.

XeoNovaDan

This program displays it at ranges up to 50 tiles, using whatever combination of accuracies you please.

While you could use the mouse-over, this can pretty much offer theoretical scenarios, which may be useful for people trying to balance mods. I'll probably add more functionality to this, but for now it's in its basic 3 functions

b0rsuk

#5
You openly admit the program is amateurish. I suppose you're not interested in other people giving you programming advice ? It's hard to comment on a program that's all in binary.

I tried to use it, but ran into exceptions. I didn't know the exact format for providing weapon accuracy. I don't like the fact you've chosen an interactive interface. I would do a commandline interface instead, where you give a couple of arguments to the program and it spits out output. sys.argv instead of some sort of scanf-like function. Unless your program draws a graph or something, I can't justify 12 MB !

XeoNovaDan

You probably put the "%'s" into the shooting accuracy values?

I currently don't know how to do exception handling as it stands. When I tested it, I had absolutely zero trouble. As for the file size, it's ridiculous how the compilation ridiculously increases file size from only 7 KB (across all 3 scripts) to 12 MB.

What I've done is pretty much all I know how to code at the moment.

GiantSpaceHamster

I recommend putting the source up on GitHub. That way people can see the latest and greatest version of the code and propose changes for you to pull (or not) into the baseline. Plus you get issue tracking which is useful for improvements and bugs.

I have not looked at your source but the most common cause of unexpected executable bloat for new developers is the inclusion of dependencies. Basically, you wrote some code, but that code depends on other libraries you didn't write, which the compiler includes in your executable.

b0rsuk

No, I tried giving all 4 accuracy values at the same time. I have since managed to use the program successfully.

There's no point for listing Medium or Long ranges for weapons like Pump Shotgun, which has a range of 16 tiles (short is 15).

I'll write my own, but I need to learn some more details about post-processed curves etc.

b0rsuk

Just a quick note, if you want an useful program don't make user memorize too much unnecessary data. When I think about shooting accuracy, I think "how accurate would a colonist with skill 14 be with Heavy SMG during rain". And I think input data should look similar. Then you can expand with extra complications like manipulation/consciousness penalty, weapon quality...

You should keep weapon data in a separate file and make the program load it. Keep it in an easy to understand format, like .csv or .json . csv is very simple and very good if you don't have too many fields and always have the same number of fields. But it starts to fall apart if you have plenty occasional fields, like 'weapon miss radius' on minigun and other special cases. .json is potentially more human readable, can support any number of fields and handle special cases very well. It also looks very similar to Python literals for data structures.

XeoNovaDan

#10
I'll give it a shot some time down the line. I'll probably end up making two separate versions of this script: Normal which is the one you suggested, and advanced which is the current script. At least until I can reliably do two modes.

Also, I've just moved the source to github. Thanks for that suggestion GiantSpaceHamster!

b0rsuk

Thanks for uploading to Github.

You can do several things to improve the structure of your code, making it more readable and idiomatic.

Minor point is that snake_case is more common variable and function naming convention than CamelCase (or camelCase). On the other hand, names starting with capital letters are often used for class names, so when people see SomeFunction, they might assume it's a class and be confused. Especially that objects can be callable. So people reading your 'Main.py' are going to assume that ShooterAccuracy and  WeaponAccuracy you're importing are classes.

https://github.com/XeoNovaDan/RimWorld-Accuracy-Calculator-v0.1/blob/master/WeaponAccuracy.py#L20
https://github.com/XeoNovaDan/RimWorld-Accuracy-Calculator-v0.1/blob/master/ShooterAccuracy.py#L6

Instead, you can write


weap_acc = [''] * 50


What you're doing is initializing a list containing an empty string, then multiplying that list by 50, so it becomes a list containing 50 empty strings. It's one of the few examples where Python is not strict about strong typing. Sometimes you accidentally multiply a list by number without knowing it's a list, which can lead to bugs. But for cases like yours the feature is very, very convenient. You don't have to count how many empty strings are there.

Second, you need to learn about python's for...in loop, which is nice because it lets you forget about manual indexes for the most part. Using your own counter variables often leads to off-by-one errors. 'for' loop in python iterates once per item in iterable (because it doesn't have to be a collection, a sequence, or anything ordered, or can be dynamically generated).

When you don't have anything to iterate over, but you want to iterate a specific number of times, you can use the for loop together with the range function. range(10) returns an iterator equivalent to [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]. Note the endpoint is not included, so in the mathematical notation range(10) would be equivalent to <0, 10) or <0, 9>. Range can be given an extra argument to tell it where it starts: range(start, stop). Normally it's treated as range(0, stop)

To wrap it up, you can rewrite your lines:
https://github.com/XeoNovaDan/RimWorld-Accuracy-Calculator-v0.1/blob/master/WeaponAccuracy.py#L25


# here, range generates 1, 2, 3, 4
for count in range(1, 5):
    weap_acc[count - 1] = "%.3f" % weap_rng[0]
    c_str = str(count)


Note when using 'range', you don't have to manually update the 'count' variable every iteration.
I understand that "c_str" is modified in every iteration because of the currently commented out line that you sometimes toggle to debug your program.

Similarly, you can rewrite this section:
https://github.com/XeoNovaDan/RimWorld-Accuracy-Calculator-v0.1/blob/master/WeaponAccuracy.py#L31


# 16, because range doesn't give the endpoint
for count in range(5, 16):
    weap_acc[count - 1] = "%.3f" % (weap_rng[0] + (acc_delta[0] * (count - 4)))
    c_str = str(count)


Lines 36, 37 seem redundant in your code and are certainly not needed in the new lines.

Save "while" loop for things which can't be simply expressed by a for loop, such as a 'do...while' loop, or when you use non-trivial sentinel variables.

https://github.com/XeoNovaDan/RimWorld-Accuracy-Calculator-v0.1/blob/master/Main.py#L15
You can rewrite this line as:


if choice in ('pawn', 'weapon', 'both'):


https://github.com/XeoNovaDan/RimWorld-Accuracy-Calculator-v0.1/blob/master/Main.py#L65

In all the 3 cases, you call Exit(). So move it out of the 'if' section, put it right at the end of the file. No need to repeat yourself.

XeoNovaDan

#12
Thanks for the suggestions for helping optimise this code :)

I'll probably come back to this once all functionality is in, as this code is technically in Alpha. I'm currently working on implementing modes which is, as you can probably guess, just adding an additional defined function for each function of the calculator, and a new 'mode' variable for the main script. I've moved the display code to a 4th script, and I'm doing sub-functions in additional scripts.

So far, I've got 'normal' and 'advanced' modes somewhat functional for weapon accuracy, but I still need to do variables and functions to handle quality, and health (or at least quality).  I think in terms of doing internal 'graphs' for post-processing shooting accuracy and health's impact on weapon accuracy, that part is going to be very poorly optimised with my skill. I've got a loose idea about it (which probably won't work), but like the rest of this existing code: it's not going to exactly be the most efficient solution and therefore poorly optimised, but it should work (if it does).

b0rsuk

If you don't optimize your code for clarity, you will end up writing long, clunky solutions, and spend more time debugging. Cleaning up and regularly is in your best interest because it lets you see more, quickly spot patterns and write better code.

XeoNovaDan

#14
Updated to v0.2

Update mainly consists of usability improvements, separation of functions, and optimisations where possible.

Quote from: b0rsuk on March 03, 2017, 07:10:39 PM
When I think about shooting accuracy, I think "how accurate would a colonist with skill 14 be with Heavy SMG during rain". And I think input data should look similar. Then you can expand with extra complications like manipulation/consciousness penalty, weapon quality...

All of the aforementioned features are now possible with the 'normal' mode, including the factoring of colonist capacities, traits (if applicable), weapon quality and even weapon HP