Design: New SLIC functions

[The Big Mc]

D'oh got add slave working only problem is that it gives you one slave and also a pop.

[Martin Gühmann]

Originally posted by The Big Mc
D'oh got add slave working only problem is that it gives you one slave and also a pop.

And what's the code?

-Martin

[The Big Mc]

there’s a problem after 7 to 9 turns the game spontaneously crashes. However I don't know if this is because of my add slave function or something that gets don’t by something I meddled with within my code.. the wired thing is it those not mater if I add 1 slave or 5 it always crashes around there.

[Martin Gühmann]

Originally posted by The Big Mc
there�s a problem after 7 to 9 turns the game spontaneously crashes. However I don't know if this is because of my add slave function or something that gets don�t by something I meddled with within my code.. the wired thing is it those not mater if I add 1 slave or 5 it always crashes around there.

Anyway I would still like to see the code. And of course the part of the log preceeding the crash.

-Martin

[The Big Mc]

for slicfunc.cpp

SFN_ERROR Slic_Addslaves::Call(SlicArgList *args)
{
	
	if(args->m_numArgs != 2)
		return SFN_ERROR_NUM_ARGS;
	Unit city;
	BOOL res;

	res = args->GetCity(0, city);
	if(!res) {
		return SFN_ERROR_TYPE_ARGS;
	}

	sint32 count;
	if(!args->GetInt(1, count))
		return SFN_ERROR_TYPE_ARGS;

		city = g_slicEngine->GetContext()->GetCity(0);

	if(!g_theUnitPool->IsValid(city)) {
		return SFN_ERROR_OK;
	}

	MapPoint pos; 
	city.GetPos(pos);	
	sint32 i;
	if(count > 0) {
		for (i=0; iChangePopulation(1);
				int owner = city.CD()->GetOwner();
				city.CD()->AddSlaveBit(owner);
				city.CD()->ChangeSpecialists(POP_SLAVE, 1);
				}
	} else {
		for(i = count; i < 0; i++) {
		}
	}
		sint32 delta_martial_law;
	CityData *cd = city.GetData()->GetCityData();
	cd->GetHappy()->CalcHappiness(*cd, FALSE, delta_martial_law, TRUE);

	return SFN_ERROR_OK;
}

and slcifunc.h

SLICFUNC(SFR_INT,  Addslaves);

there it is. i compiled it to test it on the actoscrap original.

[Martin Gühmann]

Well one thing I wonder is why it returns SFN_ERROR_OK if the city is invalid. The other thing is why do you add always a pop when you just want to add a slave and what should the empty for loop if the given count is negative, I think there are times when you want to remove one but not all slaves. And actual what do you want with the for loops at all, the ChangeSpecialists function also accepts other values then 1 even negative values. And another thing is, that the slic function should also have a third parameter that gives the POP_TYPE. POP_TYPE is an enum and here it is:

enum POP_TYPE {
	POP_WORKER,
	POP_SCIENTIST,
	POP_ENTERTAINER,
	POP_FARMER,
	POP_LABORER,
	POP_MERCHANT,
	POP_SLAVE,
	POP_MAX
};

Each element it is numbered the first is 0 and the following is one bigger so your slave has the number 6. Of course you should check whether your third parameter is out of range. Well of course in this case it would be a ChangeSpecialist function.

-Martin

[The Big Mc]

I re looked at my code and cut out a lot of the rubbish.

SFN_ERROR Slic_Addslaves::Call(SlicArgList *args)
{
	
	if(args->m_numArgs != 2)
		return SFN_ERROR_NUM_ARGS;
	Unit city;
	BOOL res;

	res = args->GetCity(0, city);
	if(!res) {
		return SFN_ERROR_TYPE_ARGS;
	}
	sint32 count;
	if(!args->GetInt(1, count))
		return SFN_ERROR_TYPE_ARGS;

	city = g_slicEngine->GetContext()->GetCity(0);
	if(count > 0) {
		city.CD()->ChangePopulation(count);
		city.CD()->ChangeSpecialists(POP_SLAVE, count);
	}
	return SFN_ERROR_OK;
}

However it still causes a crash.

[Martin Gühmann]

Originally posted by The Big Mc
However it still causes a crash.

And it is still not as universal as I suggested. You still add a pop at the same time, you have a slic function that does this. And I still cannot decide which kind of pop I like to change.

Well and now come to your problem, why do you assign a city again, even if you already got that city from the parameters, espeacilly, if you give that city a zero argument.

And another thing for namings of functions. It is easer to see what the function is called if you use: Slic_AddSlaves instead of Slic_Addslaves or should I pick a meaning: Slic_AddsLaves?

-Martin

[The Big Mc]

I do not do camel case

as for adding a pop before making it into a slave you have to or you get an error massage.

[Martin Gühmann]

Originally posted by The Big Mc
I do not do camel case

First question is why, espeacilly all the others are like that?
Second question is then why using capitals at all?
Third questions why no underscores then?

Originally posted by The Big Mc
as for adding a pop before making it into a slave you have to or you get an error massage.

So what is the error message your adds laves function produces then? I mean there must be a reason for it.

-Martin

[The Big Mc]

The error massage.
debug assertion failed!

Expression: workercount() >= delta

it goes on to point out this section of code

void CityData::ChangeSpecialists(POP_TYPE type, sint32 delta)
{
	Assert(type != POP_WORKER); 
	if(type == POP_WORKER)
		return;

	if(delta > 0) {
		Assert(WorkerCount() >= delta);
		if(WorkerCount() < delta)
			return;
	} else if(delta < 0) {
		Assert(SpecialistCount(type) + delta >= 0);
		if(SpecialistCount(type) + delta < 0)
			return;
	}

	m_numSpecialists[POP_WORKER] -= (sint16)delta;
	m_numSpecialists[type] += (sint16)delta;

	if(type == POP_SLAVE) {
		if(m_numSpecialists[POP_SLAVE] <= 0)
			m_slaveBits = 0;
	}

	AdjustSizeIndices();

	if(g_network.IsHost()) {
		g_network.Block(m_owner);
		g_network.Enqueue(this);
		g_network.Unblock(m_owner);
	} else if(g_network.IsClient()) {

		if(this == m_home_city.CD()) {
			g_network.SendAction(new NetAction(NET_ACTION_SET_SPECIALISTS, m_home_city.m_id, type, m_numSpecialists[type]));
		}
	}
}

[Martin Gühmann]

Originally posted by The Big Mc
The error massage.
debug assertion failed!

Expression: workercount() >= delta

it goes on to point out this section of code

Well that looks like you have to do first a range check, whether there are enough workers to turn into specialists. That's of course done in the slic function once you have retrieved the count value. And I thing you should give them an out of range error if there are not enough workers.

-Martin

[The Big Mc]

but martin as you point out a always add a worker before converting them into a slave.

[Martin Gühmann]

Originally posted by The Big Mc
but martin as you point out a always add a worker before converting them into a slave.

Of course it should work afterwards you changed it according to my advice in the post after the code of the last version. But I don't think this function is very useful.

1st: It is limited to slaves.
2nd: It also adds a pop point, even if I don't want to add an pop point at the same time.
3rd: I cannot remove with it a slave.

With my suggestions you don't have these limitations. But of course if you don't add at the same time a pop you have to check whether there are enough workers for slave convertion first. And if you want to remove some specialist you have to be sure that there are not less specialists you want to remove. The way to check this is given in the ChangeSpecialists function.

-Martin

[The Big Mc]

Martin I understand you point but slaves are used in a different way to specialist.

A newbie on the slic front will look for an addslave function to add or remove a slave.

However the next function I will do is a change specialist which I will be able to do very fast ones I nailed the problem in the add slave function or I made modify this one. But you can’t expect me to learn the ways of the source with a foot long section of code as easy as a 5 liner.

As for my problem it starts when the city adds a pop by growing (but not it seams when the cheat mode ads one).