UFO:Alien Invasion
Development => Coding => Topic started by: andrenal on August 05, 2012, 12:42:33 am
-
I am new to forum things and I really would like to contribute with UFO:AI development as a old x-com fan and as a c++ programmer. I analyze the game mechanics I realize that production math was broke. Doesn't make sense to produce anything if you pay the buy price at the end of production.
The production profit is always negative, seems to me the initial balance math ignores the worker cost involved or is unfinished.
So I am working on it here: https://docs.google.com/spreadsheet/ccc?key=0ApJnXtKiBSyMdGVwZWkwZWhyaU5aenY2MTRlT29UWlE#gid=0 (https://docs.google.com/spreadsheet/ccc?key=0ApJnXtKiBSyMdGVwZWkwZWhyaU5aenY2MTRlT29UWlE#gid=0) to balancing the production values for basic itens, dont know if this work will be usefull or it is already done just waiting some game improvement. I just need to do it to calm down my OCD :-[. The values I am working on doesn't unbalance the game as you can see and give some production options with a minimal profit.
I have a doubt I can't confirm yet. Does the "Cost per Item" get his value from "price" in /ufos/weapons_human.ufo?
To make this work is necessary a separate value as "production_cost" but I couldn't find where is it yet.
Can someone give me a hand? I don't know the right place to this post too ;D.
-
Hi,
Actually the code for different produce price is there already, I just didn't have time to figure out the right values. Also the designers don't really wanna players make much profit from producing because it's not the aim of the game.
My idea was if I (or someone else) could implement a dynamic market model, that could help here too. -- What I'm speaking about is if the player sells much laser pistols it's sell price go down so after a time it won't worth producing it for money.
Okay, back to the topic: You can set productioncost property for items (which are producible), if it isn't set the (market) price is used.
-geever
-
Actually the code for different produce price is there already, I just didn't have time to figure out the right values. Also the designers don't really wanna players make much profit from producing because it's not the aim of the game.
I imagine that, did you see the spreadsheet? With 30 workers you will make about 27000c per month, it's a little better that get a landed harvester and sell.
I can make a draft of values and time production for all itens, making some itens a little profitable and some dont.
If you could give a look at my numbers and give me a feedback I can take care of this.
My idea was if I (or someone else) could implement a dynamic market model, that could help here too. -- What I'm speaking about is if the player sells much laser pistols it's sell price go down so after a time it won't worth producing it for money.
I think in something like that too, and some changes in interface to improve market experience. There is a draft of that in the second tab in the spreadsheet.
Okay, back to the topic: You can set productioncost property for items (which are producible), if it isn't set the (market) price is used.
Oh =D tyvm for the help! One more thing, where is set the productioncost? research.ufo or weapons.ufo? The producetime is at research.ufo I can put there too?
-
I imagine that, did you see the spreadsheet? With 30 workers you will make about 27000c per month, it's a little better that get a landed harvester and sell.
I can make a draft of values and time production for all itens, making some itens a little profitable and some dont.
If you could give a look at my numbers and give me a feedback I can take care of this.
H-Hour did balancing in campaign. I think I'll ask him to check the values.
I think in something like that too, and some changes in interface to improve market experience. There is a draft of that in the second tab in the spreadsheet.
(H-Hour's) Buy/Sell screen on UI2 (http://ufoai.org/wiki/index.php/File:Ui2_market_window.jpg)
Oh =D tyvm for the help! One more thing, where is set the productioncost? research.ufo or weapons.ufo? The producetime is at research.ufo I can put there too?
weapons_*.ufo and likes. It's part of the object (objDef_t) data structure not the research (technology_t) one. (These data structures need revising but they're so much core part of the game it's a though scary task).
-geever
-
(H-Hour's) Buy/Sell screen on UI2 (http://ufoai.org/wiki/index.php/File:Ui2_market_window.jpg)
Nice work =)
This new UI will be implemented in 2.5?
-
Nice work =)
This new UI will be implemented in 2.5?
We hope. Theres already a wip version in the 2.5 git version (not featured in the nightly builds) you can try out.
-
Where I can set worker's salary?
Paying 4500c per month it's hard to balance ^^.
I tried find in files "4500" but nothing of workers =/
-
Where I can set worker's salary?
Paying 4500c per month it's hard to balance ^^.
I tried find in files "4500" but nothing of workers =/
camnpaign.ufo
-geever
-
Ok, that is why I couldn't find ^^
salary {
soldier_base 1500
soldier_rankbonus 0
worker_base 500
worker_rankbonus 0
scientist_base 1000
scientist_rankbonus 0
pilot_base 1500
pilot_rankbonus 0
robot_base 7500
robot_rankbonus 0
aircraft_factor 1
aircraft_divisor 25
base_upkeep 20000
admin_initial 1000
admin_soldier 75
admin_worker 75
admin_scientist 75
admin_pilot 75
admin_robot 150
debt_interest 0.005
}
How worker_base 500 become 4500 and scientist_base 1000 become 3000? Can you help me?
-
How worker_base 500 become 4500 and scientist_base 1000 become 3000? Can you help me?
You have 9 workers and 3 scientists? From where you get these values (4500 and 3000) ?
-geever
-
You have 9 workers and 3 scientists? From where you get these values (4500 and 3000) ?
I calculate from the statistics employees costs, I check costs in statistics, hire one worker, check statistics again to see the diference on value.
Like this:
Statistics -> Total Employees = 253000 with 20 workers, 16 scientists, 16 soldiers and 2 pilots
Hire 1 worker
Statistics -> Total Employees = 257500 with 21 workers, 16 scientists, 16 soldiers and 2 pilots
Don't remember the exact numbers of each type, but something pretty close to this.
I will check it again with prints when I come home.
When I said 4500, this is a salary of 1 month of a worker and 3000 is a salary of an scientist in standart and easy dificults tests. Thats why I look for 4500 on files.
My question was about some recalc on that base, because I cant understand relation with my values.
-
Ookay, it is buggy. Thanks!
You are playing on a different difficulty level than normal probably on one worker_base is not set. In this case the game using defaults hardcoded in cp_parse.cpp:
/* some default values */
s = &cp->salaries;
s->base[EMPL_SOLDIER] = 3000;
s->rankBonus[EMPL_SOLDIER] = 500;
s->base[EMPL_WORKER] = 3000;
s->rankBonus[EMPL_WORKER] = 500;
s->base[EMPL_SCIENTIST] = 3000;
s->rankBonus[EMPL_SCIENTIST] = 500;
s->base[EMPL_PILOT] = 3000;
s->rankBonus[EMPL_PILOT] = 500;
s->base[EMPL_ROBOT] = 7500;
s->rankBonus[EMPL_ROBOT] = 1500;
...
it's still not 4500 but 3000. What is that +1500? It's rank bonus. Well, workers shouldn't have ranks, but technically have one. That one should still not increase the salary, but it's buggy.
medals.ufo has the rank definitions:
rank scientist
...
rank pilot
...
rank ugv
...
rank worker
...
rank rifleman
...
... as they get indexed from zero, rank worker is IDX=3 which is the rest of that 4500 salary ( 3 * 500 = 1500 ).
What we should do is:
1. Eliminate those hardcoded defaults. My idea is to set them to -1 in the code and check the values after the parser and drop campaign definitions which don't have the necessary values set. (for this we need to set reasonable values to campaign.ufo for other difficulty levels too)
2. Fix the Rankbonus logic, it's completely broken. Rankbonus shouldn't rely on the order of the definitions in the ufo file, rather use it's factor property or so.
Would you like to take these easy bug fixes? It's good for a newcomer who yet learns UFO:AI code.
-geever
-
Ookay, it is buggy. Thanks!
Now I dont know if I am glad to help or if I get sad for stopping my Production's Balancing ...
You are playing on a different difficulty level than normal probably on one worker_base is not set. In this case the game using defaults hardcoded in cp_parse.cpp:
it's still not 4500 but 3000. What is that +1500? It's rank bonus. Well, workers shouldn't have ranks, but technically have one. That one should still not increase the salary, but it's buggy.
... as they get indexed from zero, rank worker is IDX=3 which is the rest of that 4500 salary ( 3 * 500 = 1500 ).
Are u a professor? ^^
Your explanation and didactics are very nice. =)
What we should do is:
1. Eliminate those hardcoded defaults. My idea is to set them to -1 in the code and check the values after the parser and drop campaign definitions which don't have the necessary values set. (for this we need to set reasonable values to campaign.ufo for other difficulty levels too)
2. Fix the Rankbonus logic, it's completely broken. Rankbonus shouldn't rely on the order of the definitions in the ufo file, rather use it's factor property or so.
Would you like to take these easy bug fixes? It's good for a newcomer who yet learns UFO:AI code.
-geever
YES! =D but ... there are some considerations, I dont have too much free time to dive in the project this month cause I have to finish a home reform by myself. =/
This will be my first contact with opensource development team work paradigm. So I have to be prepared to dont shame myself. =)
I am concerned about the salary variation over difficulty. I think there are more ways to set things harder than change salarys, cause once production is balanced, change salarys will invalidate the work for other dificult levels.
For example, the sell value and Nations payment can be diminished instead. Because a worker salary is a production cost variable.
-
I believe campaigns now have a producerate variable, which can be used to make production quicker or slower across all items.
-
I believe campaigns now have a producerate variable, which can be used to make production quicker or slower across all items.
H-Hour, could you set up salaries for all difficulty levels? As I see it's missing at almost every but main. Probably someone thought "it will be the same as main if not set" but it's not true.
-geever
-
I believe campaigns now have a producerate variable, which can be used to make production quicker or slower across all items.
Don't you think we could have at standart difficult a balanced production with production costs near buy price, sometimes with a little profit, sometimes without but still good to produce but not to sell?
H-Hour, could you set up salaries for all difficulty levels? As I see it's missing at almost every but main. Probably someone thought "it will be the same as main if not set" but it's not true.
-geever
geever, what did you offer to me to fix, scripts or source code? I understood source code ...
-
H-Hour, could you set up salaries for all difficulty levels? As I see it's missing at almost every but main. Probably someone thought "it will be the same as main if not set" but it's not true.
Not at the moment. I am only able to check internet in brief occassional moments for a few weeks.
-
geever, what did you offer to me to fix, scripts or source code? I understood source code ...
The code. For the scripts we can either copy the current hardcoded defaults or choose some reasonable.
I just want to eliminate the hardcoded part.
-geever
-
1. Eliminate those hardcoded defaults. My idea is to set them to -1 in the code and check the values after the parser and drop campaign definitions which don't have the necessary values set. (for this we need to set reasonable values to campaign.ufo for other difficulty levels too)
It's simple, yes. The only thing i don't like in this - many copy/paste in campaign.ufo so much more thing to not forget to change for balance. Right now soldier's salary is 1500 in normal (rankbonus 0) and 3000 on easy (rankbonus 500 from hardcoded values). :)
2. Fix the Rankbonus logic, it's completely broken. Rankbonus shouldn't rely on the order of the definitions in the ufo file, rather use it's factor property or so.
Added rank_no to medals.ufo and using it instead of index. Problem is rank and employee type stored in different structures. Without redesign there is unnecessary search for parent structure to get employee type.
Calling debug_listhired in console crashes game after listing soldiers.
I'm not familiar with patches but tried to implement changes from above. Side effect - promotion of soldiers in existing saves by 4 ranks. And there is no any check for that.
-
Err... Nobody interested?
-
Were the patches uploaded last time either? Nevermind.
You're mixing the meaning of the index (Idx) of ranks which isn't good.
You should create an own function:
rank_t *RANK_GetByLevel (employeeType_t type, int level)
(rank_no should be "level" imho)
Also you should read our coding guidelines (http://ufoai.org/wiki/index.php/Coding_guidelines).
-geever
-
You should create an own function:
rank_t *RANK_GetByLevel (employeeType_t type, int level)
The question is if we should change meaning of chr.score.rank. If no then
rank_t *CL_GetRankByIdx (const int index)just useless, if yes then new function like
rank_t *CL_GetRankLevelByIdx (const int index)required. Anyway my implementation is not perfect :(
(rank_no should be "level" imho)
No problem.
Also you should read our coding guidelines (http://ufoai.org/wiki/index.php/Coding_guidelines).
Can you say most typical mistake?
Coding tips & FAQ (http://ufoai.org/forum/index.php/topic,64.0.html) really lacks info about using git and specially about work with patches and stash.
-
The question is if we should change meaning of chr.score.rank. If no then
rank_t *CL_GetRankByIdx (const int index)just useless,
chr.score.rank should point to an exact rank, while level (or rank_no) is to define a promotion order between ranks. So the meaning of chr.score.rank should not be changed BUT CL_GetRankByIdx is not useless either. We wanna hide the underlying datastructure.
If we use &ccs.ranks At time we change the datastructure we will need to search and replace all these references. If we use CL_GetRankByIdx, only the body of that function needs to be changed.
I've already put changing ranks datastructure to a linked list on my personal wishlist so it's not limited to an artificial number (32 now).
You should also not forget, that level is scripted, so 2 (or more) ranks can have the same level (even for the same employee category) unless we put extra check on that in ranks parser. But why would we? I think it's more flexible to allow more ranks of the same level maybe with different requirements and let the system choose randomly on promotion from which are met.
Can you say most typical mistake?
Mostly whitespace errors. It makes easier to read the code if you separate arguments of a funtion and so, like
- const rank_t *rank = CL_GetRankByIdx(chr->score.rank,EMPL_SOLDIER);
+ const rank_t *rank = CL_GetRankByIdx(chr->score.rank, EMPL_SOLDIER);
- for (int i=0;i<ccs.numRanks;i++)
- if (ccs.ranks[i].rank_no==index && ccs.ranks[i].type==type)
+ for (int i = 0; i < ccs.numRanks; i++)
+ if (ccs.ranks[i].rank_no == index && ccs.ranks[i].type == type)
Also, please put comments in separate lines before the thing it describes.
And check the gudeline of the Git commit comment:
* summary of the commit
* some detail notes if you feel like
The emplty line is important so as the asterisk at each line.
Coding tips & FAQ (http://ufoai.org/forum/index.php/topic,64.0.html) really lacks info about using git and specially about work with patches and stash.
The article Getting the source (http://ufoai.org/wiki/index.php/Getting_the_source) contain some information about using Git.
Your patch was about right but... there is always a but :)
That two patches should be two but not in this way. As you fixed problems in the first patch with the second, they should be merged so it's easier to understand - you can use interactive rebasing for that: git rebase -iBut you put two different things in the first commit:
* the campaign parameters patch
* and (part of) the ranks patch
These should be splitted. Interactive rebasing can help in this one too.
Git help: http://www.kernel.org/pub/software/scm/git/docs/git-rebase.html ;)
one more about the code:
+ s->base[EMPL_SOLDIER] = -1;
+ s->rankBonus[EMPL_SOLDIER] = -1;
+ s->base[EMPL_WORKER] = -1;
+ s->rankBonus[EMPL_WORKER] = -1;
+ s->base[EMPL_SCIENTIST] = -1;
+ s->rankBonus[EMPL_SCIENTIST] = -1;
+ s->base[EMPL_PILOT] = -1;
+ s->rankBonus[EMPL_PILOT] = -1;
+ s->base[EMPL_ROBOT] = -1;
+ s->rankBonus[EMPL_ROBOT] = -1;
You could/should use a loop on employeeTypes for things like this. Shorter, better.
-geever
-
Thanks for detailed answer. That's what i wanted.
chr.score.rank should point to an exact rank
But that's integer value which fully depends on order ranks is parsed. Maybe it should be saved/loaded as string (rank name) and used as integer only at run time?
The article Getting the source (http://ufoai.org/wiki/index.php/Getting_the_source) contain some information about using Git.
Yes, there is some info about getting source. But here is not enough info about how contribute it back.
And i always wonder why you don't use std::vector, std::list, etc. Specially vector. This should be great help for storing data with same access speed as static array.
-
OK, patches rewritten. I didn't touched promotion logic but it should be changed. It still relies on fact that higher rank have higher index.
-
And i always wonder why you don't use std::vector, std::list, etc. Specially vector. This should be great help for storing data with same access speed as static array.
First, the whole project just got converted to C++ from C.
Second, STL containers are pointer unsafe: they can and will move data around, requiring you to have an extra level of indirection. Not to mention the extra fun of dynamically allocating memory in the middle of pseudo-realtime code (and we are CPU limited already). So for us, in many cases they are slower than static arrays.
Third, unless you typedef them, all the code turns into unreadable clutter of <>::
Said that, some of the code really could use STL containers. For example, replacing our linked list implementation with std::list probably could be useful; at least we can be sure that it is 100% debugged (we had problems with our implementation before, like crashes or data corruption in case of empty list).
-
OK, patches rewritten. I didn't touched promotion logic but it should be changed. It still relies on fact that higher rank have higher index.
Your first patch have been added to master. It had a few whitespace errors but corrected. As I see there is still a few things in campaign_t that have hardcoded defaults.
The second patch is two patches actually. You're right on that ranks' id should be saved, not the index, but implementation is not fully correct. You shouldn't include a cp_ stuff from outside campaign code. The different game modes can be compiled to be different DLL/SO modules and such cross reference could make problems with linking.
There is two ways to resolve this issue
- Make ranks a campaign only by moving chr.score.rank into the employee datastructure and savesystem
- Make ranks common by moving the rank handling (cp_ranks.cpp/h) outside of campaign
I've discussed the matter with Mattn and H-Hour before and chose the second.
The first part of your second patch about rank levels is in master too, but not yet the save stuff.
-geever
-
The second patch is two patches actually.
It was five patches merged in one. But how it was supposed to be divided?
- Make ranks common by moving the rank handling (cp_ranks.cpp/h) outside of campaign
Move files to cgame directory, rename files accordingly, correct inclusions, move ranks data from campaign to cl_rank.cpp as static var, eliminate remaining direct data access in other parts of game. Is that all? Oh, and something wrong with making - it still searches cp_rank.cpp in old location.
-
I didn't rename rank functions. Or i should replace CL_ with GAME_ ?
And i'm not sure that i did all required modifications to makefiles. Game is building fine and running.
-
nice - but we should go one step futher. the rank code should be something where the game code has access to, too - not only the cgame. this isn't a client-only feature (in the future at least)
so maybe move this close to e.g. chr shared code?
-
so maybe move this close to e.g. chr shared code?
Do you mean move to game directory? I can of course but you should remember that rank parsing code relies on some parts of campaign code. Namely global variable cp_campaignPool and function E_GetEmployeeType.
-
Rank list removed from campaign data structure. But it continue using campaign memory pool + campaign had implicit reset of its own data. Both results in corruption of rank data because ranks list for some reason is loaded twice at least: on each save subsystem initialization. But second results in freeing memory pool (with rank IDs, etc) without notification of ranks list hence without resetting ranks counter and getting trash in rankID, rank name etc.
So added CL_ResetRanks function and its call in CP_ResetCampaignData.
To use ranks from game part:
* ranks list must have its own memory management (i've tried assigned by caller)
* employeeType and some simple related functions should be defined in game part (i've tried and got huge mess with headers inclusions)
-
If i understand it right... Rank code now fully independent from campaign code.
There is some additional work required to use rank code in game part.
-
now game contains a file named cl_ - this should not be the case. cl_ is for the client.
i will check the remaining of the patch asap
-
now game contains a file named cl_ - this should not be the case. cl_ is for the client.
Renamed files and functions. As far as i can see all works fine but it's in game part only in sense as files in directory. For real use in game part some more high level work required. I.e. you should decide how it will be shared with client side.
-
the data should be stored in the csi_t struct at least - but i can tell more if i checked the patches.
thanks a lot already.
-
I don't want to be annoying but patch with reset function in this post (http://ufoai.org/forum/index.php/topic,6890.msg55236.html#msg55236) should be applied ASAP to current master. It provide fix to possible saves corruption due to trash from memory written as soldier rank id (introduced in already applied my patches). Or other fix should be applied, like return rank table in campaign data structure but that invalidate other my patches.