changed all ocurrences of action_list to action_group and act_list to act_group
Reply To dark-ether
however action_list_iterate is being used
I've not looked the patch yet, but aren't those (or at least some of them) iterations supposed to happen over all actions, custom or not. So all actions should be in the same list, no separate constructs for custom and internal actions (as much as there is "internal" actions in the long term plans - I don't know how much they are supposed to get generalized away).
Reply To cazfi
Reply To dark-ether
however action_list_iterate is being used
I've not looked the patch yet, but aren't those (or at least some of them) iterations supposed to happen over all actions, custom or not. So all actions should be in the same list, no separate constructs for custom and internal actions (as much as there is "internal" actions in the long term plans - I don't know how much they are supposed to get generalized away).
i am not certain but i think no, there is a action_iterate function which seems to be for all actions iteration, and in the code i looked at using action_list_iterate it seems to iterate over a array of stored actions id so it probably is iteration over a subgroup of all actions. but even then i haven't changed much, just renamed all action_list and act_list ocurrences to action_group and act_group if i haven't missed anything, it should continue working exactly the same as before the patch. this patch is a preparatory one for when i create a speclist instance for actions
Reply To dark-ether
Reply To cazfi
Reply To dark-ether
however action_list_iterate is being used
I've not looked the patch yet, but aren't those (or at least some of them) iterations supposed to happen over all actions, custom or not. So all actions should be in the same list, no separate constructs for custom and internal actions (as much as there is "internal" actions in the long term plans - I don't know how much they are supposed to get generalized away).
i am not certain but i think no, there is a action_iterate function which seems to be for all actions iteration, and in the code i looked at using action_list_iterate it seems to iterate over a array of stored actions id so it probably is iteration over a subgroup of all actions. but even then i haven't changed much, just renamed all action_list and act_list ocurrences to action_group and act_group if i haven't missed anything, it should continue working exactly the same as before the patch. this patch is a preparatory one for when i create a speclist instance for actions
i went and to check and yes it is exactly this action_iterate is for all actions, action_list_iterate(with this patch being renamed to action_group_iterate) for some actions, if you look at their definition in actions.h action_iterate starts with id = 0 and goes up to id = NUM_ACTIONS-1 and action_group_iterate receives a array of actions ids and iterate over them. so when i add extra actions i will have to add another loop for the "extra actions", about long term plans i can't know what Sveinung planned but at least for me all actions should eventually be unhardcoded, and to do so temporalily there will be a a hardcoded action list and a custom action one and we gradually move actions from being hardcoded to being custom.
Couple of things:
I mostly agree with everything you said, and am mostly thankful that you decided to respond anyway even considering all the problems in my patches.
When i created this patch i was unaware that i could change git history as long as no one had received it yet,and wrongly assumed it was impossible without even thinking about it.
I do agree that action group is a bad name, going of your suggestion i would propose NTArrray, short for none terminated. I also considered Carrray as it similarly to a c string is terminated by the empty value but that could be confusing. i am open to further suggestions.
I will provide a new, better patch soon.
Sorry for my English.
Regarding discussion which would you prefer, that i create a ticket or forum post to discuss something i am planning or should i just create a ticket when i implemented something with a patch?
For instance after you mentioned that linked lists are undesirable on the game struct. I am planning on creating a wrapper for dynamic arrays. When i have an idea like this should i
4 seems wrong, and i am leaning towards 2, obviously i am only going to have some discussion tickets open at a time ,so would that be okay? specifically i plan 2 discussion tickets one that is my current overarching goal and another on what i am specifically doing in the moment.
Reply To dark-ether
I am planning on creating a wrapper for dynamic arrays.
We already have dynamic arrays (though we call 'em "vectors"); see to comment at the top of utility/specvec.h for info, and the places where it's included for examples (a very prominent one being the requirement_vector).
However: We also don't generally use those for ruleset data, in favor of statically allocated arrays with a fixed maximum size. If I had to guess, the original reason for this is that we need that maximum size for the way the networking code handles arrays – the packet structs come with allocated space for the maximum possible array size. So we need to have a maximum possible number of e.g. unit types, or technologies, or custom actions; and when we have that, there's really no point in complicating the ruleset structure by using anything other than a statically allocated array.
(Expanding the networking code to be able to work with specvecs would be awesome, of course, but that's a whole 'nother thing entirely. It would also open the door for a number of issues about whose responsibility is freeing those specvecs.)
Reply To dark-ether
I do agree that action group is a bad name, going of your suggestion i would propose NTArrray, short for none terminated. I also considered Carrray as it similarly to a c string is terminated by the empty value but that could be confusing. i am open to further suggestions.
On further review, I do think "array" should be fine – it's not like there are any other sensible values to terminate it anyway, and if it was an array with sizing information added, well, you'd probably be using a vector for that, not a plain array. So I don't think there's significant risk of confusion.
Reply To dark-ether
When i have an idea like this should i
1. create a forum post to discuss
2. create a ticket to discuss
3. create a discussion in another platform i don't know about yet
4. not discuss and only post a patch when complete
4 seems wrong, and i am leaning towards 2, obviously i am only going to have some discussion tickets open at a time ,so would that be okay? specifically i plan 2 discussion tickets one that is my current overarching goal and another on what i am specifically doing in the moment.
For larger features, there's often a "meta-ticket" that tracks a number of related tickets, and where general discussion about how to go about it (and what new sub-tickets we need) can happen. For things that show up on their own, it can be helpful to start a ticket in a way that is entirely problem- or goal-oriented (i.e. what we want to achieve), and then figure out / discuss how to go about solving the problem / achieving the goal there.
Forum is of course a bit better-suited to long-form discussion, and reaches more people – especially for things that have direct effects outside the code, open questions about how something should be represented in ruleset files etc., that can be useful, since most people probably don't follow everything that happens over here. On the other hand, it splits discussion across two sites, which makes it harder to follow, requires people to register for another account, and which increases the likelihood of parts of the discussion being lost if one site goes down / has its database corrupted / is moved to a new backend without the option to migrate the data / etc.
There is also an IRC channel, I am told (the information is somewhere on the website or the wiki, I think?), but I can't attest to how active that is, seeing as I myself haven't even so much as looked in an IRC client's general direction in years.
Reply To alienvalkyrie
There is also an IRC channel, I am told (the information is somewhere on the website or the wiki, I think?), but I can't attest to how active that is, seeing as I myself haven't even so much as looked in an IRC client's general direction in years.
If someone goes there, let me know. The couple of times I've been on the channel after the move to Libera.CHAT, there has been no activity at all, so I rarely bother to start up the client myself. Of course it might be that somebody else is also checking it occasionally when I'm not there and also seeing as totally dead place.
For development related discussion that does not quite fit any ticket, the best option would probably be freeciv-dev mailing list. Forums might be an option in some cases where end-users may want to participate. freeciv-dev: https://www.freelists.org/list/freeciv-dev
my definition of soon probably needs some work but here it is
Looks good.
A note for the future: Commit messages should typically refer to the issue they're related to, e.g. "See osdn#44084"; if you do that already, it saves us the effort of having to amend the commit.
(There's no need to submit a new patch here, since I've already changed it – this is just for future reference.)
for adding custom actions it will be necessary to store information about the actions, the method i thought about would be creating a speclist for action structs and changing the structs to store all necessary information about the actions. however action_list_iterate is being used, so it is necessary to change it. also having various lists function that aren't related to the lists would be confusing so i changed every ocurrence for the currently existing functions from list to group.i used ripgrep to find all files with either action_list or act_list and then on each of these files used sed to change action_list to action_group and act_list to act_group.i then used git diff to find all the differences and if it looked if it shouldn't be changed i reverted the change. example: the ChangeLog File i also added a rule_name field to the action struct.