Pink Petal Games

Game Editing And Additions => User Mods => Topic started by: h1262216 on March 21, 2019, 06:33:17 PM

Title: some code improvements
Post by: h1262216 on March 21, 2019, 06:33:17 PM
Hello,seeing that work seems to continue on WM, I thought I would also provide a few patches for things I noticed when looking over the source code. These should (for now) not change any of the games behaviour, but simply make the code a bit more robust and modern.
The first replaces some uses of plain c strings with std::strings to make FileList a bit more readable, and does some small other code modernizations (=default, nullptr etc).The second reduces the amount of temporary objects that are allocated on the heap a bit. It seems that the code often generates a customer, girl etc, which is constructed by new and deleted at the end of the function again. These are replaced by stack variable in the patch. However, since this changes every occurence of "->" into ".", the patch file is quite large.

If you like the direction, I can supply a few more.
Title: Re: some code improvements
Post by: aevojoey on March 22, 2019, 10:09:35 AM
There seems to be a problem with attaching files to posts here.
Can you upload them to mega.nz or google docs or somewhere else and link them here?
Or you can push it to github and make a pull request. https://github.com/crazywm/crazys-wm-mod (https://github.com/crazywm/crazys-wm-mod)
Title: Re: some code improvements
Post by: h1262216 on March 24, 2019, 12:13:39 PM
OK, I've made pull requests.
Btw., it would be nice if the repository were a little more cleaned up. In particular, the fact that the main directory contains two folders whose names are 'crazys-wm-mod' and ' crazys-wm-mod' is rather annoying.
In a second step, putting all source files in a subfolder would also improve the structure of the repo.
I would provide a PR, but I am not sure how these changes would affect the existing vs project files, since I dont use vs.
Title: Re: some code improvements
Post by: aevojoey on March 24, 2019, 12:56:46 PM
OK, I've made pull requests.
Btw., it would be nice if the repository were a little more cleaned up. In particular, the fact that the main directory contains two folders whose names are 'crazys-wm-mod' and ' crazys-wm-mod' is rather annoying.
In a second step, putting all source files in a subfolder would also improve the structure of the repo.
I would provide a PR, but I am not sure how these changes would affect the existing vs project files, since I dont use vs.Greetings
I got them merged but I did them in the wrong order so in cInventory.cpp the int to long change got undone.
I have fixed in on my end so the next time I upload it will be correct.
Title: Re: some code improvements
Post by: h1262216 on March 24, 2019, 03:50:12 PM
cool, thanksI've made another one that tries to get rid of as many usages of the gGirls global variable as possible. In particular any calls to skills, traits, and stats are now done using the Girl interface instead of this global.
Title: Re: some code improvements
Post by: aevojoey on March 24, 2019, 04:42:21 PM
cool, thanksI've made another one that tries to get rid of as many usages of the gGirls global variable as possible. In particular any calls to skills, traits, and stats are now done using the Girl interface instead of this global.
Thanks, I have wanted to do this for a long time but there was sooooooo much of it so I never got around to it.

g_Girls.HasItem has no girl->has_item
Title: Re: some code improvements
Post by: h1262216 on March 25, 2019, 02:08:10 PM
Yes, it was quite the search and replace endeavour.I have pushed the next set of changes, that removes all the methods of `AbstractGirls` and uses the `girl->` interface for them.
Title: Re: some code improvements
Post by: aevojoey on March 26, 2019, 11:39:41 AM
Yes, it was quite the search and replace endeavour.I have pushed the next set of changes, that removes all the methods of `AbstractGirls` and uses the `girl->` interface for them.
Ok, I got that in.

The EndOfDay changes break the game so I will have to redo that.
Title: Re: some code improvements
Post by: h1262216 on March 27, 2019, 06:20:27 AM
Hey, I am looking at the traits code, and have a few questions: * In sRandomGirl::process_trait_xml, a new sTrait* is created based on the traits name. Shouldn't this be looked up from the global traits list g_Traits? Otherwise the trait won't have its description and type set. (also, the current implementation has a potential memory leak: If no name is given in the xml, the newly allocated sTrait is not deleted again)* what is the purpose of cTraits::getTranslateName? The traits are loaded dynamically from an xml, but translations are hardwired into the c++ file?
Title: Re: some code improvements
Post by: aevojoey on March 27, 2019, 10:16:41 AM
Hey, I am looking at the traits code, and have a few questions: * In sRandomGirl::process_trait_xml, a new sTrait* is created based on the traits name. Shouldn't this be looked up from the global traits list g_Traits? Otherwise the trait won't have its description and type set. (also, the current implementation has a potential memory leak: If no name is given in the xml, the newly allocated sTrait is not deleted again)* what is the purpose of cTraits::getTranslateName? The traits are loaded dynamically from an xml, but translations are hardwired into the c++ file?
getTranslateName is there because there were some old girl packs that were hand written so capitalization was not always consistent.
This is old code and can probably be removed.

process_trait_xml is also old code.
It loads the random trait list from rgirls and creates the list to choose from.
It is only needed to get chances for a rgirl to have a trait so the full trait stuff is not needed here.
Though fixing the memory leak should be done.
Title: Re: some code improvements
Post by: aevojoey on March 27, 2019, 12:15:19 PM
I have pushed the changes for .06.04.00 to github.
The changes you made to the End Day were removed so you will want to back those up before fetching the new code.
Title: Re: some code improvements
Post by: h1262216 on March 27, 2019, 04:19:08 PM
the nice thing about git is that the information in still there :)Your changes make the project no longer compile on linux, because std::sort is not available until you #include <algorithm>.

I have made a PR for another set of changes, this time regarding the traits. Instead of using an implicit linked list by having pointers in the traits class, cTraits now stores all traits in a std::list.Also, I have (re)added effects to sTrait, so that the traits can automatically apply a malus/bonus to stats, skills and enjoyment. The values are laoded from the *.traitsx file, so these changes manage to removeabout 1000 lines of the c++ code.
I would like to continue in this direction, but I feel like further changes to traits require a bit more effort and planning. For example, I tried automatic the mutual exclusion of traits, but could not because there are a few special cases in there (e.g. changes due to the "No Nipples" trait seem to ignore the `rememberflag` parameter. Is that deliberate?
Ideally, I would like to achieve the following:A TraitSpec class, that know the specification of a trait. This includes name and description, and it effects (currently only offsets to stats, skills, enjoyment; is there something else that should be set here?).
A Trait class, that is the trait actually applied to a girl. This saves whether the trait is temporary and how many days are left, whether it is remembered, and whether it is active.
All these traits together are stored in a TraitCollection class, that provides a convenient interface for adding and removing a trait, as well as ensuring that no mutually exclusive traits are set.
Finally, the static arrays for traits and remembered traits that are currently in sGirl will be replaced by TraitCollection. This also removes any restriction on the number of traits a girl can have (is that restriction there because of the static arrays, or is it actually a deliberate decision to not allow more than a certain amount of traits per girl?).
Any thoughts / comments on that?
Title: Re: some code improvements
Post by: h1262216 on March 28, 2019, 04:40:50 PM
I think I may have found a bug in the traits implementation:
sGirl::add_trait may call g_Girls.ApplyTraits. This resets all trait stat modifications in girl->m_StatTr, girl->m_SkillTr[r], girl->m_EnjoymentTR[r]  (what about m_TrainingTr? Is that used at all at the moment?)Stat modifications are then applied using UpdateStatTrHowever, this does modify the actual stats, instead of the trait ones, for certain stat.
Code: [Select]
if (stat == STAT_HEALTH || stat == STAT_HAPPINESS || stat == STAT_TIREDNESS || stat == STAT_EXP ||
stat == STAT_LEVEL || stat == STAT_HOUSE || stat == STAT_ASKPRICE)  {
girl->upd_stat(stat, amount);         return; }
So the modifications of the actual stat will be repeated every time the stat influences are updated. Or am I missing something?
Title: Re: some code improvements
Post by: aevojoey on March 31, 2019, 06:31:42 PM
I think I may have found a bug in the traits implementation:
sGirl::add_trait may call g_Girls.ApplyTraits. This resets all trait stat modifications in girl->m_StatTr, girl->m_SkillTr[r], girl->m_EnjoymentTR[r]  (what about m_TrainingTr? Is that used at all at the moment?)Stat modifications are then applied using UpdateStatTrHowever, this does modify the actual stats, instead of the trait ones, for certain stat.
Code: [Select]
if (stat == STAT_HEALTH || stat == STAT_HAPPINESS || stat == STAT_TIREDNESS || stat == STAT_EXP ||
stat == STAT_LEVEL || stat == STAT_HOUSE || stat == STAT_ASKPRICE)  {
girl->upd_stat(stat, amount);         return; }
So the modifications of the actual stat will be repeated every time the stat influences are updated. Or am I missing something?
I see what you mean, it is a bug but has little effect on the game.
STAT_HOUSE and STAT_ASKPRICE are recalculated most of the times they are asked for so they are not a problem.
STAT_HEALTH, STAT_HAPPINESS and STAT_TIREDNESS change so often that they would not make much of an impact on the game.
STAT_EXP and STAT_LEVEL don't have any traits that modify them.
Title: Re: some code improvements
Post by: h1262216 on April 05, 2019, 08:10:45 AM
did you take a look at the other PR I made (regarding the traits code)?
Title: Re: some code improvements
Post by: aevojoey on April 07, 2019, 11:08:47 AM
did you take a look at the other PR I made (regarding the traits code)?
I started to look at it but RL got in the way. I'm looking at it now.
Title: Re: some code improvements
Post by: aevojoey on April 11, 2019, 01:03:12 PM
Can please create a how to guide for building the game in linux?
What do you use to edit and compile the game?
While I have been using linux for years, I have not really gotten too deep into the coding side of it. I use Linux Mint.
Title: Re: some code improvements
Post by: h1262216 on April 14, 2019, 04:55:22 PM
I use CLion as an IDE. I has a great set of tools for refactoring code, but unfortunately it is not free.
As a build system (technically a build system generator) I use CMake.

On Linux Mint, the library dependencies (SDL, SDL_image, etc.) can be installed via apt, and CMake can find them automatically. I can attach my CMakeLists.txt file in the next PR.