[Freeciv-Dev] [patch #3721] Getting rid of move_type dependency on unitselect dialog

[Marko Lindqvist]

Update of patch #3721 (project freeciv):

 Assigned to: jtn => None   
 Planned Release:2.6.0, 3.0.0 => 3.0.0  

___

Follow-up Comment #7:

Unit select dialogs are refactored in 3.0 development, so targeting this one
there.

___

Reply to this item at:

  

___
  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 #3721] Getting rid of move_type dependency on unitselect dialog

[Marko Lindqvist]

Update of patch #3721 (project freeciv):

 Planned Release:   2.6.0 => 2.6.0, 3.0.0   


___

Reply to this item at:

  

___
  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 #3721] Getting rid of move_type dependency on unitselect dialog

[Marko Lindqvist]

Update of patch #3721 (project freeciv):

 Planned Release:   2.5.0 = 2.6.0  


___

Reply to this item at:

  http://gna.org/patch/?3721

___
  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 #3721] Getting rid of move_type dependency on unitselect dialog

[Emmet Hikory]

Follow-up Comment #6, patch #3721 (project freeciv):

One of the patches in patch #4866 provides a new ruleset field (unit class
group) that could be used for this purpose, if there is interest in that sort
of solution (as hinted at the end of the original submission).

___

Reply to this item at:

  http://gna.org/patch/?3721

___
  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 #3721] Getting rid of move_type dependency on unitselect dialog

[Emmet Hikory]

Follow-up Comment #4, patch #3721 (project freeciv):

Not being thread safe isn't just about whether the client is multithreaded. 
The entire computational system (client+server) has at least two threads, and
if I read unit_move() correctly, send_unit_info_to_onlookers() is called a few
times before the transported units are moved, so that there is a window during
which the client sees the transporting unit moved and the transported units
not yet moved, because the client thread (the single thread of the client
process) isn't waiting for the server to finish with unit_move_handling() to
do the next action.  In practice, the situation described above would require
either extremely variable network latency or a vast disparity in computational
power between the client and the server, but this definitely conceivable with
a fast gaming PC running the client and a heavily-loaded public internet
server running on aging donated hardware in the lowest QoS band of some
oddly-connected data centre.

Note that in the situation above, although the units are on different tiles,
they still believe themselves to be transported, so by querying transport
status, the information collected can be expected to also be reliable once the
server has finished processing the move (and since the information that the
transport is no longer occupied is sent before the information that the unit
has moved, the opposite case (thinking a unit is in a transport when it has in
fact moved out) will never occur with the current code).

___

Reply to this item at:

  http://gna.org/patch/?3721

___
  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 #3721] Getting rid of move_type dependency on unitselect dialog

[Marko Lindqvist]

Follow-up Comment #5, patch #3721 (project freeciv):

Oh, you were talking about server/client sync. Yes, even though client is
single-threaded, and in that sense does a lot of things atomically, user
action can happen between any two packets from server.
As for transport and cargo being in different tiles, that can certainly happen
- we've just been fixing entire class of bugs when client does (even without
user interaction) illegal things while that's the case.

___

Reply to this item at:

  http://gna.org/patch/?3721

___
  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 #3721] Getting rid of move_type dependency on unitselect dialog

[Marko Lindqvist]

Follow-up Comment #3, patch #3721 (project freeciv):

While making things thread-safe in case threads are used in the future does
have it merits, I don't see value in partial solution that just has smaller
window for particular error, but is still not at all thread-safe. That doesn't
make it one bit easier to implement it thread-safely in the future.

___

Reply to this item at:

  http://gna.org/patch/?3721

___
  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 #3721] Getting rid of move_type dependency on unitselect dialog

[Emmet Hikory]

Follow-up Comment #2, patch #3721 (project freeciv):

I don't think it is safe to use !can_exist_at_tile() as the filter if
we're looking to exclude transported units: that implies an assertion that at
any time the client is sufficiently idle to take a new command, no
untransported unit cannot exist on it's tile, which wouldn't be thread-safe if
units are drowning, or possibly in certain transport situations (I haven't
reviewed the client code sufficiently to confirm this: it may be that the
client already group-moves units the server permits to move, or similar). 
Additionally, units in nested transport may be selected if they are native,
even if they cannot be used (e.g. Cruise Missile in Mech. Inf. in Transport). 
If the intent is to find units that are both transported and unable to
disembark, we should be using something like:

ptrans = unit_transport_get(punit);
if (ptrans != NULL
 !(can_unit_unload(punit, ptrans)
  can_unit_exist_at_tile(punit, utile))) {
  return FALSE;
}

Using tile_city(utile) != NULL in both conditions causes any units in
cities to appear twice.  Perhaps these conditions could depend instead on the
local terrain+infrastructure, so:

LAND: (!is_ocean(utile)  is_native_tile(unit_type(punit), utile)))
SEA: (is_ocean(utile)  is_native_tile(unit_type(punit), utile)))

This still causes some duplication (some units may be native to both,
either inherently, or due to roads/bases/etc.), but doesn't automatically
select all sea units in harbour when selecting LAND.  On the other hand, it
will make non-native (e.g. BuildAnywhere) units unselectable, which may be
confusing.

The above said, I don't use this selector other than for tile range, so
these opinions may be ill-informed.

___

Reply to this item at:

  http://gna.org/patch/?3721

___
  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 #3721] Getting rid of move_type dependency on unitselect dialog

[Marko Lindqvist]

Update of patch #3721 (project freeciv):

 Assigned to:None = jtn

___

Follow-up Comment #1:

Giving to jtn for him to decide if this fits to his select dialog plans.

___

Reply to this item at:

  http://gna.org/patch/?3721

___
  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 #3721] Getting rid of move_type dependency on unitselect dialog

[Marko Lindqvist]

URL:
  http://gna.org/patch/?3721

 Summary: Getting rid of move_type dependency on unitselect
dialog
 Project: Freeciv
Submitted by: cazfi
Submitted on: Tue 19 Feb 2013 09:27:44 AM EET
Category: client
Priority: 5 - Normal
  Status: Ready For Test
 Privacy: Public
 Assigned to: None
Originator Email: 
 Open/Closed: Open
 Discussion Lock: Any
 Planned Release: 2.5.0

___

Details:

Unit select dialog uses unit move_type, which we've tried to retire from
non-ai code almost a decade already (since gen-movement development begun).

Attached patch replaces individual checks for move types with generic
can_unit_exist_at_tile() check.

This changes behavior in many ways. It's probably matter of taste if these
behavioral changes are for better or worse:

- Both selection type removed entirely. In most rulesets Both units mean
air units, in some also amphibious units. So you no longer be able select just
that selection of units.
- Land and Sea selection methods required both tile to be of the selected
type, and the unit to have that move type. Terrain is still limited the same
way, but units are subject to above mentioned can_unit_exist_at_tile() - 1)
Also Both units belong to selection 2) tile must really be native to unit,
e.g., wheeled land units are not in selection in roadless tiles, alien ruleset
native terrain limitations apply
- World used to select all units. Now units are subject to
can_unit_exist_at_tile() so transported units are not part of the selection


 If this is not sufficient, we may use some other property of units than
move_type to make the decision what selection groups they belong to. Unit
class is one possibility, but suffers from that separate classes like
Trireme and Sea could then not be combined. Maybe we need to introduce new
ruleset field for ruleset author to classify units, either to hardcoded
categories or dynamic ones.


 If we accept this patch to TRUNK (I'd go with it at least as temporary
solution), should it go also to S2_4?



___

File Attachments:


---
Date: Tue 19 Feb 2013 09:27:44 AM EET  Name: UnitselectNativity.patch  Size:
4kB   By: cazfi

http://gna.org/patch/download.php?file_id=17256

___

Reply to this item at:

  http://gna.org/patch/?3721

___
  Message sent via/by Gna!
  http://gna.org/


___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev