[Freeciv-Dev] [patch #3879] Remove negated reqs{} field in favor of something semantically nicer
Update of patch #3879 (project freeciv): Status: Ready For Test = Done Open/Closed:Open = Closed ___ Reply to this item at: http://gna.org/patch/?3879 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #3879] Remove negated reqs{} field in favor of something semantically nicer
Follow-up Comment #4, patch #3879 (project freeciv): This discussion appears to be complete. Using a positive entry, as this appears to be preferred by 3/3 of those expressing a preference. Selected the term present, as this was in the preference lists of 2/2 of those expressing a preference. Rebasing above current trunk now includes several more ruleset changes to reflect SVN revision 22794. The attached patch is stacked above patch #3835 as it includes changes to code changed by that patch, but this is mostly for convenience (not needing to post an update there). The relevant changes could be moved without significant impact to functionality. (file #17936) ___ Additional Item Attachment: File name: rename-negated-to-present.patch Size:49 KB ___ Reply to this item at: http://gna.org/patch/?3879 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #3879] Remove negated reqs{} field in favor of something semantically nicer
Follow-up Comment #5, patch #3879 (project freeciv): This discussion appears to be complete. Yes. This is also rather important change to get in... The attached patch is stacked above patch #3835 as it includes changes to code changed by that patch, but this is mostly for convenience ...so I would prefer to drop the patch #3835 dependency, and to go forward with this (#3879) while #3835 is still a bit undecided. ___ Reply to this item at: http://gna.org/patch/?3879 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #3879] Remove negated reqs{} field in favor of something semantically nicer
Follow-up Comment #6, patch #3879 (project freeciv): Updated patch attached against SVN 22836. (file #17938) ___ Additional Item Attachment: File name: rename-negated-to-present+SVN22836.patch Size:47 KB ___ Reply to this item at: http://gna.org/patch/?3879 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #3879] Remove negated reqs{} field in favor of something semantically nicer
Follow-up Comment #7, patch #3879 (project freeciv): With most terms the problem seems to be that they don't make it clear that FALSE - requirement is not allowed to be present, rather than just being not really a requirement. Idea for potential future ticket: What if it wasn't boolean (TRUE/FALSE) in ruleset format, but the strings used there were present/absent, which ruleset loading code then would just convert to internal boolean representation? ___ Reply to this item at: http://gna.org/patch/?3879 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #3879] Remove negated reqs{} field in favor of something semantically nicer
Update of patch #3879 (project freeciv): Category:rulesets = general Status:None = Ready For Test Assigned to:None = cazfi Planned Release: = 2.6.0 ___ Reply to this item at: http://gna.org/patch/?3879 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #3879] Remove negated reqs{} field in favor of something semantically nicer
Follow-up Comment #8, patch #3879 (project freeciv): Further experimentation showed that this patch broke effects network packets. Updated patch attached with sense flipped for generated packets. As a side note, it appears that effects requirements could not previously handle negated correctly in the client, which bug is preserved with the migration to present, to be handled in another future ticket. (file #17944) ___ Additional Item Attachment: File name: rename-negated-to-present+effects.patch Size:47 KB ___ Reply to this item at: http://gna.org/patch/?3879 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #3879] Remove negated reqs{} field in favor of something semantically nicer
Follow-up Comment #2, patch #3879 (project freeciv): present, active, exists, condition, test, asserted, found With most terms the problem seems to be that they don't make it clear that FALSE - requirement is not *allowed* to be present, rather than just being not really a requirement. Though people should realize that there's no point in requirement that is not required... If the above terms I prefer present and active (ones that I thought up myself before seeing your list) and would add required to the list (despite the required requirement oddity) along with simple true (I think this carries a bit of the message that FALSE = not true - false) ___ Reply to this item at: http://gna.org/patch/?3879 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #3879] Remove negated reqs{} field in favor of something semantically nicer
Follow-up Comment #3, patch #3879 (project freeciv): Seeing true makes me think of boolean: it makes the C code a little odd (bool boolean, if (preq-boolean), boolean ? boolean : ), but may be easy to understand for ruleset authors. ___ Reply to this item at: http://gna.org/patch/?3879 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #3879] Remove negated reqs{} field in favor of something semantically nicer
URL:
http://gna.org/patch/?3879
Summary: Remove negated reqs{} field in favor of something
semantically nicer
Project: Freeciv
Submitted by: persia
Submitted on: Wed 24 Apr 2013 09:52:01 AM JST
Category: rulesets
Priority: 5 - Normal
Status: None
Privacy: Public
Assigned to: None
Originator Email:
Open/Closed: Open
Discussion Lock: Any
Planned Release:
___
Details:
The semantics of the negated field in requirements really irritate me. The
conditionals in the code generally require reverse polarity and
double-negative thinking. Ruleset fragments need careful thought because
TRUE means that a given requirement must not be met, and FALSE means that
it must be met.
There are two ways to address this, either by enabling nreqs everywhere (as
suggested in patch #3332), or by changing the word used to be something
positive. I've prepared patches achieving both, the first against trunk, and
the second against trunk + patch #3835 for consideration. I don't believe
either is currently perfect for application to trunk, but thought discussion
might be improved with code to review. Although the attached patches compile,
I have not tested them in gameplay at all.
I personally prefer the use of a positive term, as this better integrates with
my work-in-progress towards disjunctive requirements specification, but have
not successfully figured out a good word to use for this in the last few weeks
of thinking about it: candidates have been present, active, exists,
condition, test, asserted, found, and others that I dismissed before
adding them to my TODO documentation (hence the use of the easily
searchreplaceable term in the patch). Suggestions welcomed gratefully.
Note that the introduction of nreqs uses an entirely different mechanism than
that suggested in patch #3332 (part of why I'm not reusing that ticket):
specifically reversing the polarity of ruleset defined nreqs when storing them
in the reqs vector, rather than introducing nreqs vectors everywhere (with the
need for all the attendant plumbing in requirements.c and code review of
callers). This doesn't address the double-negation in the code, but it does
mask it from ruleset authors to some degree, making rulesets easier to both
write and read.
___
File Attachments:
---
Date: Wed 24 Apr 2013 09:52:01 AM JST Name:
0001-Drop-negated-in-favor-of-nreqs.patch Size: 13kB By: persia
http://gna.org/patch/download.php?file_id=17821
---
Date: Wed 24 Apr 2013 09:52:01 AM JST Name:
0001-Rename-negated-to-something-positive.patch Size: 37kB By: persia
http://gna.org/patch/download.php?file_id=17822
___
Reply to this item at:
http://gna.org/patch/?3879
___
Message sent via/by Gna!
http://gna.org/
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #3879] Remove negated reqs{} field in favor of something semantically nicer
Follow-up Comment #1, patch #3879 (project freeciv): or by changing the word used to be something positive. I agree. Note that the introduction of nreqs uses an entirely different mechanism than that suggested in patch #3332 The mechanism was wrong and my original idea was confuse. My purpose was the patch #1342, where requirement_vector nreqs replaced obsoleted_by. This was the second step. The first step was in patch #1339, where requirement_vector reqs replaced the fields require_advance, need_improvement and need_government. Same for patch #1341 and #1340. ___ Reply to this item at: http://gna.org/patch/?3879 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
