#45602: is_..._in_range() with a standardized fingerprint
Open Date: 2022-09-07 17:44
Last Update: 2022-10-23 21:18
URL for this Ticket:
https://osdn.net//projects/freeciv/ticket/45602
RSS feed for this Ticket:
https://osdn.net/ticket/ticket_rss.php?group_id=12505&tid=45602
---------------------------------------------------------------------
Last Changes/Comment on this Ticket:
2022-10-23 21:18 Updated by: alienvalkyrie
Comment:
Question is what that fingerprint should be.
The option closest to what we currently have would be
enum fc_tristate is_foo_in_range(const struct req_context *context, const struct player *other_player, enum req_range range, bool survives, universals_u value) { const struct foo *pfoo = value.foo; /* determine if foo is in range */ }with the universals_u union passed directly (since I'm reasonably sure declaring each function to just take the type it wants would be very risky).
On the other end of the spectrum, a more compact (function-type-wise) option would be
enum fc_tristate is_foo_in_range(const struct req_context *context, const struct player *other_player, const struct requirement *req) { fc_assert_ret_val(req->source.kind == VUT_FOO, TRI_MAYBE); const struct foo *pfoo = req->source.value.foo; enum req_range range = req->range; /* determine if req is active, *ignoring* req->present */ }but that would lead to a lot of boilerplate between assertions and unpacking the data that every variant will need.
If we go with the latter, it might also be sensible to rename the functions as is_foo_req_active.
---------------------------------------------------------------------
Ticket Status:
Reporter: cazfi
Owner: (None)
Type: Patches
Status: Open
Priority: 5 - Medium
MileStone: (None)
Component: (None)
Severity: 5 - Medium
Resolution: None
---------------------------------------------------------------------
Ticket details:
Looking to make is_req_active()
I think the main problem is that those 'case's in the switch are different from each other. The compiled code really need to go "if else if else if else if else if else" to find the block for the requirement type they are interested about. It would be much better to have all cases to call similar callback function -> then we could have those callbacks in an array indexed by the requirement type id.
With the new req_context this makes even more sense than before. is_req_active() can just pass the context pointer as it got it, to the callbacks. There's no "passing parameters needed by the particular callback"
The req_context point makes it clear that the change won't get backported to S3_0, where we don't have that context.
--
Ticket information of Freeciv project
Freeciv Project is hosted on OSDN
Project URL: https://osdn.net/projects/freeciv/
OSDN: https://osdn.net
URL for this Ticket:
https://osdn.net/projects/freeciv/ticket/45602
RSS feed for this Ticket:
https://osdn.net/ticket/ticket_rss.php?group_id=12505&tid=45602